CSHARP-5529: Optimize grouping.First().X to not retrieve the entire $$ROOT #1653
Conversation
rstam
added
the
improvement
adelinowona
left a comment
adelinowona
left a comment
There was a problem hiding this comment.
Looks good but you should update the LINQ2 tests as well since the optimization changes the output. Evergreen is failing because of that.
I was wondering if that would be the case. Updating now. |
sanych-sun
left a comment
sanych-sun
left a comment
There was a problem hiding this comment.
LGTM + minor change suggested.
| { | ||
| internal static class AstExpressionExtensions | ||
| { | ||
| public static bool IsConstant(this AstExpression expression, BsonValue value) |
There was a problem hiding this comment.
Should we rename it to IsConstantEqualsTo or make it return the constant's value instead:
public static bool IsConstant(this AstExpression expression, out BsonValue value)
| Assert(query, | ||
| 1, | ||
| "{ $group: { _id: '$A', __agg0: { $first: '$$ROOT'} } }", | ||
| "{ $group: { _id: '$A', __agg0: { $first: '$$ROOT'}, __agg1 : { $first : '$B' } } }", |
There was a problem hiding this comment.
You could argue that __agg1 is redundant because we've already fetched the entire first $$ROOT so we could compute { $first : "$B" } by calling __agg0.B client-side.
Not sure how easy this would be. Do you think it is worth working on?
There was a problem hiding this comment.
No harm in checking how easy it would be right?
There was a problem hiding this comment.
I don't think it's easy.
There was a problem hiding this comment.
Actually, I am finding a way to do this that is simpler than I thought.
I had originally thought we had to solver a tougher problem, which was: to look for redundancy between the various __aggx fields.
But in this particular case we can just generate a better accumulator expression in VisitFilterField.
| Assert(query, | ||
| 1, | ||
| "{ $group: { _id: '$A', __agg0: { $first: '$$ROOT'} } }", | ||
| "{ $group: { _id: '$A', __agg0: { $first: '$$ROOT' }, __agg1 : { $first : '$B' } } }", |
There was a problem hiding this comment.
Same comment as above about __agg1.
| "{ $group: { _id: '$A', __agg0: { $first: '$$ROOT'}, __agg1 : { $first : '$B' } } }", | ||
| "{ $match: { '__agg0.B' : 'Balloon' } }", | ||
| "{ $project: { Key: '$_id', FirstB: '$__agg0.B', _id: 0 } }"); | ||
| "{ $project: { Key: '$_id', FirstB: '$__agg1', _id: 0 } }"); |
There was a problem hiding this comment.
Same comment as above about __agg1.
|
I've rebased on See the comments for a question about whether some further work is called for. |
3 participants
No description provided.