[SERVER-54740] Extend flexibility to the MatchExpression walkers Created: 23/Feb/21  Updated: 06/Dec/22

Status: Backlog
Project: Core Server
Component/s: Querying
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major - P3
Reporter: Jacob Evans Assignee: Backlog - Query Optimization
Resolution: Unresolved Votes: 0
Labels: quick-tech-debt
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: File s-54740-walker.patch1.diff     File s-54740-walker.patch2.diff    
Issue Links:
Depends
is depended on by SERVER-59892 Convert ExpressionWithPlaceholder int... Backlog
Assigned Teams:
Query Optimization
Sprint: Query Optimization 2021-05-31
Participants:

 Description   

We have a generic walker at https://github.com/mongodb/mongo/blob/master/src/mongo/db/query/tree_walker.h which is used for MatchExpressions. It would be nice to find a way to get the flexibility improvements made by SERVER-54709 applied to both walkers or to merge them into one entity with the benefits of both.



 Comments   
Comment by Timour Katchaounov [ 11/Sep/21 ]

As agreed with anton.korshunov stopping work on this task. Once someone implements SERVER-59892, I can finish this up. Attach the current state of the task as diffs.

Comment by Timour Katchaounov [ 11/Sep/21 ]

Description of the attached patches:

  1. Replace query/tree_walker.h with pipeline/expression_walker.h.
  2. Make NotMatchExpression inherit from ListOfMatchExpression in order to make it's storage a vector, and change getChildVector to return that vector instead of nullptr.
  3. Convert the following classes: ElemMatchObjectMatchExpression, ElemMatchValueMatchExpression, ListOfMatchExpression to store their children as a vector of unique_ptr<MatchExpression>.

Probably these patches also make some other relevant changes, I don't remember. The patches should be applied in increasing order.

Comment by Timour Katchaounov [ 11/Sep/21 ]

I tried various approaches to unify the two walkers - query/tree_walker.h and pipeline/expression_walker.h.

The final approach we decided upon with jacob.evans is to:

  • Replace query/tree_walker.h with pipeline/expression_walker.h.
  • Make pipeline/expression_walker.h use a getChilVector method to access the children of the current node.
  • Make child storage of all MatchExpression classes be the same - a vector<unique_ptr<MatchExpression>>.
  • Implement getChildVector for all classes that are either missing it, or return a NULL instead of an actual vector.
    This method should return the underlying vector storage.

The last point requires a number of changes to some MatchExpression classes that store their children in a class-specific way.

All the above changes are implemented in the 3 patches attached to this Jira.

The only thing left is to convert the storage of several "InternalSchema*MatchExpression" classes to a vector. In order for this to become doable, we need to first make ExpressionWithPlaceholder a proper MatchExpression class, which is filed as a separate task - SERVER-59892.

Comment by Timour Katchaounov [ 21/May/21 ]

This task should take into account SERVER-54741.

Generated at Thu Feb 08 05:34:21 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.