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

Refactor agg expressions to use children elements instead of storing child references

    XMLWordPrintableJSON

Details

    • Icon: Improvement Improvement
    • Resolution: Fixed
    • Icon: Major - P3 Major - P3
    • 6.3.0-rc0
    • None
    • None
    • Fully Compatible
    • QO 2022-09-19, QO 2022-10-03, QE 2022-10-17

    Description

      Originally the agg Expressions hierarchy was designed to hold references to named arguments within an individual expression class. For example. It was supposed to provide faster access to expression arguments. The trade off of a hypothetical performance improvement was an increased memory footprint for agg expression trees. In our recent discoveries it was revealed that the ref approach also doesn't play nicely alongside optional expression arguments, which may result in an awkward construct of boost::optional<boost::intrusive_ptr<Expression>&> when we use boost::optional to hold a reference to something that is optional itself.

      Moreover, we already diverged from this pattern of storing refs to expression arguments and use direct array access (as in _children[0]).

      As part of this ticket we should evaluate if there is any real benefit of storing refs or otherwise switch to a different approach when we use mutable accessors:
       

      class ExpressionXyz {
      public:
          ...
          intrusive_ptr<Expression>& foo() {
              return _children[0];
          }
          intrusive_ptr<Expression>& bar() {                                         
              tassert(_children.size() > 1); // bar is optional and should never be accessed when not present
              return _children[1];
          }
      };
      

       

      Attachments

        Activity

          People

            timour.katchaounov@mongodb.com Timour Katchaounov
            anton.korshunov@mongodb.com Anton Korshunov
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: