Query: Add support for custom aggregate operators #28092
Conversation
Resolves #22957 - Introduce IAggregateMethodCallTranslator and family to translate aggregate methods - Currently we assume that either method instance or first argument (for static method) will be the one which can be of type enumerable, rest of the arguments if any are scalar. - Also refactor code in SqlTranslator.VisitMethodCall to avoid visiting grouping element source multiple times
|
Hello @smitpatel! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
|
Apologies, while this PR appears ready to be merged, I've been configured to only merge when all checks have explicitly passed. The following integrations have not reported any progress on their checks and are blocking auto-merge:
These integrations are possibly never going to report a check, and unblocking auto-merge likely requires a human being to update my configuration to exempt these integrations from requiring a passing check. Give feedback on thisFrom the bot dev teamWe've tried to tune the bot such that it posts a comment like this only when auto-merge is blocked for exceptional, non-intuitive reasons. When the bot's auto-merge capability is properly configured, auto-merge should operate as you would intuitively expect and you should not see any spurious comments. Please reach out to us at fabricbotservices@microsoft.com to provide feedback if you believe you're seeing this comment appear spuriously. Please note that we usually are unable to update your bot configuration on your team's behalf, but we're happy to help you identify your bot admin. |
| var method = methodCallExpression.Method; | ||
| var arguments = methodCallExpression.Arguments; |
There was a problem hiding this comment.
Awesome stuff!
Here are a few comments, the only important thing is the possibility to use custom aggregate functions in QueryableMethodTranslatingEV (e.g. at top-level).
I'll work on the PG side against this branch to make sure I'm not blocked by anything - will report results.
src/EFCore.Relational/Query/Internal/QueryableAggregateMethodTranslator.cs
Show resolved
Hide resolved
src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
Show resolved
Hide resolved
| Expression? result = null; | ||
| switch (methodCallExpression.Method.Name) | ||
| { | ||
| case nameof(Queryable.Average): |
There was a problem hiding this comment.
Nice to see all this gone from here 👍
src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs
Show resolved
Hide resolved
src/EFCore.Relational/Query/RelationalAggregateMethodCallTranslatorProvider.cs
Show resolved
Hide resolved
src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs
Show resolved
Hide resolved
src/EFCore.Relational/Query/Internal/QueryableAggregateMethodTranslator.cs
Show resolved
Hide resolved
ghost
merged commit 
|
Should remember to remove auto-merge... :( |
|
Ouch! |
|
If you submit a fixup I'll approve, sorry... |
…e enumerable in aggregate function Feedback on #28092
Resolves #22957