[SERVER-63552] Modify EqLookupNode in query solution to use FieldPath Created: 10/Feb/22  Updated: 29/Oct/23  Resolved: 21/Mar/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 6.0.0-rc0

Type: Task Priority: Major - P3
Reporter: Anna Wawrzyniak Assignee: Irina Yatsenko (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Sprint: QE 2022-04-04
Participants:

 Description   

EqLookupNode QSN uses string field names to represent field paths. This causes 2 problems:
1) It requires conversion from FieldPath to string when creating the node and then splitting it to the path again
2) It is misleading, and may suggest that the fields are just fields, rather than paths that require multiple getField and/or nested object creations.



 Comments   
Comment by Irina Yatsenko (Inactive) [ 21/Mar/22 ]

https://github.com/10gen/mongo/pull/3910 fixed this, it was merged with https://github.com/mongodb/mongo/commit/ebb1f3900176b9df1f3c20646f349c9785914c43.

Comment by David Storch [ 21/Mar/22 ]

irina.yatsenko was there a commit made against this ticket? I don't see a github link.

Comment by Anna Wawrzyniak [ 16/Feb/22 ]

 

ExpressionFieldPath {
    ...
    const FieldPath _fieldPath;
    const Variables::Id _variable;
};

so basically looks like ExpressionFieldPath is FieldPath + variable. And the only applicable variable would be Variables:kRootId, so this seems like an unnecessary degree of freedom, since we don't allow anything other than a FieldPath in $lookup.

I think FieldPath is the best choice.

 

Comment by Anna Wawrzyniak [ 16/Feb/22 ]

Great question. I’m not sure, it doesnt seem like there is a very domain separation ruels that we adhere too here.
both FieldPath and ExpressionFieldPath are defined in mongo/db/pipeline.

The ExpressionFieldPath is part of the Expression hierarchy, so that makes me favor standalone FieldPath a bit more, especially for the “as” field, which we want to interpret very differently than Expression evaluation (we need to assemble newObjs there, rather than evaluate expression as is. Expression comes with exectuion logic, that we will be ignoring in this case.

additionally, FieldPath already existed before group, (in the sort code) and in ReturnNode, so for archeological consistency reasons, I think it also makes sense to use it.

Comment by Irina Yatsenko (Inactive) [ 15/Feb/22 ]

Or should we store ExpressionFieldPath in EqLookupNode instead?

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