Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-79009

Remove debug calls to VariableEnvironment::rebuild

    • Type: Icon: Task Task
    • Resolution: Fixed
    • Priority: Icon: Major - P3 Major - P3
    • 7.2.0-rc0
    • Affects Version/s: None
    • Component/s: None
    • Query Optimization
    • Fully Compatible
    • QO 2023-10-02, QO 2023-10-16

      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,
       

            Assignee:
            henri.nikku@mongodb.com Henri Nikku
            Reporter:
            timour.katchaounov@mongodb.com Timour Katchaounov
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: