opt_phase_manager.cpp has several calls to VariableEnvironment::rebuild whose sole purpose is to call tassert to verify that the variable environment is in a consistent state.
These calls are costly, and should be avoided in release builds. These calls and their checks should wrapped in a kDebugBuild.
This is a sketch of the proposed change:
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 @@ -181,12 +197,14 @@ void OptPhaseManager::runMemoLogicalRewrite(const OptPhase phase, tassert(6808702, "Logical writer failed to rewrite fix point.", fixPointRewritten); input = extractLatestPlan(_memo, rootGroupId); - env.rebuild(input); + // TODO: wrap in kDebugBuild + // env.rebuild(input); } - if (env.hasFreeVariables()) { - tasserted(6808703, "Plan has free variables: " + generateFreeVarsAssertMsg(env)); - } + // TODO: wrap in kDebugBuild + // if (env.hasFreeVariables()) { + // tasserted(6808703, "Plan has free variables: " + generateFreeVarsAssertMsg(env)); + // } } PlanExtractorResult OptPhaseManager::runMemoPhysicalRewrite( @@ -247,12 +265,13 @@ PlanExtractorResult OptPhaseManager::runMemoPhysicalRewrite( result = extractPhysicalPlans(includeRejected, _physicalNodeId, _metadata, _ridProjections, _memo); - for (const auto& planEntry : result) { - env.rebuild(planEntry._node); - if (env.hasFreeVariables()) { - tasserted(6808707, "Plan has free variables: " + generateFreeVarsAssertMsg(env)); - } - } + // TODO: wrap in kDebugBuild + // for (const auto& planEntry : result) { + // env.rebuild(planEntry._node); + // if (env.hasFreeVariables()) { + // tasserted(6808707, "Plan has free variables: " + generateFreeVarsAssertMsg(env)); + // } + // } return result; } @@ -304,7 +323,10 @@ PlanExtractorResult OptPhaseManager::optimizeNoAssert(ABT input, const bool incl }; runStructuralPhases<OptPhase::ConstEvalPre, OptPhase::PathFuse, ConstEval, PathFusion>( - ConstEval{env, canInlineEvalPre}, PathFusion{env}, env, input); + ConstEval{env, canInlineEvalPre, {}, {}, false /*eagerRefTracking*/}, + PathFusion{env}, + env, + input); auto planExtractionResult = runMemoRewritePhases(includeRejected, env, input); // At this point "input" has been siphoned out. @@ -366,14 +388,19 @@ PlanExtractorResult OptPhaseManager::optimizeNoAssert(ABT input, const bool incl }; runStructuralPhase<OptPhase::ConstEvalPost, ConstEval>( - ConstEval{env, {} /*canInlineEvalFn*/, erasedProjFn, renamedProjFn}, + ConstEval{env, + {} /*canInlineEvalFn*/, + erasedProjFn, + renamedProjFn, + false /*eagerRefTracking*/}, env, planEntry._node); - env.rebuild(planEntry._node); - if (env.hasFreeVariables()) { - tasserted(6808710, "Plan has free variables: " + generateFreeVarsAssertMsg(env)); - } + // TODO: wrap in kDebugBuild + // env.rebuild(planEntry._node); + // if (env.hasFreeVariables()) { + // tasserted(6808710, "Plan has free variables: " + generateFreeVarsAssertMsg(env)); + // } } tassert(6624174,
- related to
-
SERVER-81783 Investigate a segfault when not rebuilding VariableEnvironment after memo implementation phase
-
- Backlog
-