-
Type: Task
-
Resolution: Fixed
-
Priority: Major - P3
-
Affects Version/s: None
-
Component/s: Query Planning
-
Query Optimization
-
Fully Compatible
-
QO 2023-09-18, QO 2023-10-02
The path lowering rewrite in path_lower.cpp invokes reference tracking a number of times. This seems to be unnecessary, since path lowering cannot introduce/remove variables during. Since reference tracking is quite expensive, it makes sense to delay reference tracking until the end of path lowering. The following patch illustrates the idea:
diff --git a/src/mongo/db/query/optimizer/opt_phase_manager.cpp b/src/mongo/db/query/optimizer/opt_phase_manager.cpp index 8b7b218bda0..ce3b380b13f 100644 --- a/src/mongo/db/query/optimizer/opt_phase_manager.cpp +++ b/src/mongo/db/query/optimizer/opt_phase_manager.cpp @@ -110,6 +110,22 @@ void OptPhaseManager::runStructuralPhase(C instance, VariableEnvironment& env, A !_debugInfo.exceedsIterationLimit(iterationCount)); } + // TODO: Unify the tree comments below, and add them to runStructuralPhases as well. + + // This is needed for cases in which EvalPathLowering is called from a context other than during + // PathLowering. If the ABT is modified in a way that adds variable references and definitions + // the environment must be updated. + + // This is needed for cases in which EvalFilterLowering is called from a context other than + // during PathLowering. If the ABT is modified in a way that adds variable references or + // definitions the environment must be updated. + + // During PathLowering we may call EvalPathLowering or EvalFilterLowering. These each may call + // rebuild on a subset of the ABT, which will produce invalid references for refs that point to + // definitions outside of that subset. Rebuild the tree to avoid leaving those free variables + // for the caller. + + env.rebuild(input); if (env.hasFreeVariables()) { tasserted(6808709, "Plan has free variables: " + generateFreeVarsAssertMsg(env)); } @@ -144,7 +160,7 @@ void OptPhaseManager::runStructuralPhases(C1 instance1, changed |= instance2.optimize(input); } } - + env.rebuild(input); if (env.hasFreeVariables()) { tasserted(6808701, "Plan has free variables: " + generateFreeVarsAssertMsg(env)); } diff --git a/src/mongo/db/query/optimizer/rewrites/path_lower.cpp b/src/mongo/db/query/optimizer/rewrites/path_lower.cpp index cf852899335..d3bd0ebd352 100644 --- a/src/mongo/db/query/optimizer/rewrites/path_lower.cpp +++ b/src/mongo/db/query/optimizer/rewrites/path_lower.cpp @@ -41,13 +41,6 @@ bool EvalPathLowering::optimize(ABT& n, bool rebuild) { algebra::transport<true>(n, *this); - // This is needed for cases in which EvalPathLowering is called from a context other than during - // PathLowering. If the ABT is modified in a way that adds variable references and definitions - // the environment must be updated. - if (_changed && rebuild) { - _env.rebuild(n); - } - return _changed; } @@ -233,13 +226,6 @@ bool EvalFilterLowering::optimize(ABT& n, bool rebuild) { algebra::transport<true>(n, *this); - // This is needed for cases in which EvalFilterLowering is called from a context other than - // during PathLowering. If the ABT is modified in a way that adds variable references or - // definitions the environment must be updated. - if (_changed && rebuild) { - _env.rebuild(n); - } - return _changed; } @@ -430,14 +416,6 @@ bool PathLowering::optimize(ABT& n) { algebra::transport<true>(n, *this); - // During PathLowering we may call EvalPathLowering or EvalFilterLowering. These each may call - // rebuild on a subset of the ABT, which will produce invalid references for refs that point to - // definitions outside of that subset. Rebuild the tree to avoid leaving those free variables - // for the caller. - if (_changed) { - _env.rebuild(n); - } - return _changed; }
- is duplicated by
-
SERVER-79094 Reduce calls to VariableEnvironment::rebuild from path lowering
- Closed
- related to
-
SERVER-80730 [CQF] Reduce optimization phases for sampling queries
- Closed