[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: |
| 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 ] | |||||
|
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. 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? |