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

Reduce calls to VariableEnvironment::rebuild from path lowering

    • Type: Icon: Task Task
    • Resolution: Fixed
    • Priority: Icon: Major - P3 Major - P3
    • 7.2.0-rc0
    • 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;
       }
       

            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: