Full support for custom provider aggregate functions #22957
Closed
Assignees
Labels
area-query
closed-fixed
The issue has been fixed and is/will be included in the release indicated by the issue milestone.
punted-for-6.0
type-enhancement
Milestone
Comments
|
GroupBy aggregate issue is not related to custom provider function. Not all GroupBy patterns work right now and no one can extend them either that is one of the part in #18923 For second point, just need to make TranslateExpression protected. But that issue is not related to aggregate either. Any custom queryable method will suffer that. |
|
OK, thanks - it would be nice if we could make all this work in 6.0. It seems useful to me to track here everything what's needed for custom provider aggregate functions, but up to you if you feel everything's covered elsewhere. |
This was referenced May 8, 2021
1 task
This was referenced Apr 27, 2022
smitpatel
added a commit
that referenced
this issue
May 2, 2022
Design: - Introduce `SqlEnumerableExpression` - a holder class which indicates the `SqlExpression` is in form of a enumerable (or group) coming as a result of whole table selection or a grouping element. It also stores details about if `Distinct` is applied over grouping or if there are any orderings. - Due to above `DistinctExpression` has been removed. The token while used to denote `Distinct` over grouping element were not valid in other parts of SQL tree hence it makes more sense to combine it with `SqlEnumerableExpression`. - To support dual pass, `GroupByShaperExpression` contains 2 forms of grouping element. One element selector form which correlates directly with the parent grouped query, second subquery form which correlates to parent grouped query through a correlation predicate. Element selector is first used to translate aggregation. If that fails we use subquery form to translate as a subquery. Due to 2 forms of same component, GroupByShaperExpression disallows calling into VisitChildren method, any visitor which is visiting a tree containing GroupByShaperExpression (which appears only in `QueryExpression.ShaperExpression` or LINQ expression after remapping but before translation) must intercept the tree and either ignore or process it appropriately. - An internal visitor (`GroupByAggregateChainProcessor`) inside SqlTranslator visits and process chain of queryable operations on a grouping element before aggregate is called and condense it into `SqlEnumerableExpression` which is then passed to method which translates aggregate. This visitor only processes Where/Distinct/Select for now. Future PR will add processing for OrderBy/ThenBy(Descending) operations to generate orderings. - Side-effect above is that joins expanded over the grouping element (due to navigations used on aggregate chain), doesn't translate to aggregate anymore since we need to translate the join on parent query, remove the translated join if the chain didn't end in aggregate and also de-dupe same joins. Filing issue to improve this in future. Due to fragile nature of matching to lift the join, we shouldn't try to lift joins. - To support custom aggregate operations, we will either reused `IMethodCallTranslator` or create a parallel structure for aggregate methods and call into it from SqlTranslator by passing translated SqlEnumerableExpression as appropriate. - For complex grouping key, we cause a pushdown so that we can reference the grouping key through columns only. This allows us to reference the grouping key in correlation predicate for subquery without generating invalid SQL in many cases. - With complex grouping key converting to columns, now we are able to correctly generate identifiers for grouping queries which makes more queries with correlated collections (where either parent or inner both queries can be groupby query) translatable. Resolves #27132 Introduce a pass to translate grouping element chain ending in aggregate before translating it as a subquery. Resolves #27226 Resolves #27433 Cause a pushdown when grouping key is complex so that SQL parser doesn't throw error that ungrouped columns are being referenced. Relates to #22957
smitpatel
added a commit
that referenced
this issue
May 2, 2022
Design: - Introduce `SqlEnumerableExpression` - a holder class which indicates the `SqlExpression` is in form of a enumerable (or group) coming as a result of whole table selection or a grouping element. It also stores details about if `Distinct` is applied over grouping or if there are any orderings. - Due to above `DistinctExpression` has been removed. The token while used to denote `Distinct` over grouping element were not valid in other parts of SQL tree hence it makes more sense to combine it with `SqlEnumerableExpression`. - To support dual pass, `GroupByShaperExpression` contains 2 forms of grouping element. One element selector form which correlates directly with the parent grouped query, second subquery form which correlates to parent grouped query through a correlation predicate. Element selector is first used to translate aggregation. If that fails we use subquery form to translate as a subquery. Due to 2 forms of same component, GroupByShaperExpression disallows calling into VisitChildren method, any visitor which is visiting a tree containing GroupByShaperExpression (which appears only in `QueryExpression.ShaperExpression` or LINQ expression after remapping but before translation) must intercept the tree and either ignore or process it appropriately. - An internal visitor (`GroupByAggregateChainProcessor`) inside SqlTranslator visits and process chain of queryable operations on a grouping element before aggregate is called and condense it into `SqlEnumerableExpression` which is then passed to method which translates aggregate. This visitor only processes Where/Distinct/Select for now. Future PR will add processing for OrderBy/ThenBy(Descending) operations to generate orderings. - Side-effect above is that joins expanded over the grouping element (due to navigations used on aggregate chain), doesn't translate to aggregate anymore since we need to translate the join on parent query, remove the translated join if the chain didn't end in aggregate and also de-dupe same joins. Filing issue to improve this in future. Due to fragile nature of matching to lift the join, we shouldn't try to lift joins. - To support custom aggregate operations, we will either reused `IMethodCallTranslator` or create a parallel structure for aggregate methods and call into it from SqlTranslator by passing translated SqlEnumerableExpression as appropriate. - For complex grouping key, we cause a pushdown so that we can reference the grouping key through columns only. This allows us to reference the grouping key in correlation predicate for subquery without generating invalid SQL in many cases. - With complex grouping key converting to columns, now we are able to correctly generate identifiers for grouping queries which makes more queries with correlated collections (where either parent or inner both queries can be groupby query) translatable. Resolves #27132 Introduce a pass to translate grouping element chain ending in aggregate before translating it as a subquery. Resolves #27266 Resolves #27433 Cause a pushdown when grouping key is complex so that SQL parser doesn't throw error that ungrouped columns are being referenced. Relates to #22957
|
Base infra added in #27931 |
smitpatel
added a commit
that referenced
this issue
May 3, 2022
Design: - Introduce `SqlEnumerableExpression` - a holder class which indicates the `SqlExpression` is in form of a enumerable (or group) coming as a result of whole table selection or a grouping element. It also stores details about if `Distinct` is applied over grouping or if there are any orderings. - Due to above `DistinctExpression` has been removed. The token while used to denote `Distinct` over grouping element were not valid in other parts of SQL tree hence it makes more sense to combine it with `SqlEnumerableExpression`. - To support dual pass, `GroupByShaperExpression` contains 2 forms of grouping element. One element selector form which correlates directly with the parent grouped query, second subquery form which correlates to parent grouped query through a correlation predicate. Element selector is first used to translate aggregation. If that fails we use subquery form to translate as a subquery. Due to 2 forms of same component, GroupByShaperExpression disallows calling into VisitChildren method, any visitor which is visiting a tree containing GroupByShaperExpression (which appears only in `QueryExpression.ShaperExpression` or LINQ expression after remapping but before translation) must intercept the tree and either ignore or process it appropriately. - An internal visitor (`GroupByAggregateChainProcessor`) inside SqlTranslator visits and process chain of queryable operations on a grouping element before aggregate is called and condense it into `SqlEnumerableExpression` which is then passed to method which translates aggregate. This visitor only processes Where/Distinct/Select for now. Future PR will add processing for OrderBy/ThenBy(Descending) operations to generate orderings. - Side-effect above is that joins expanded over the grouping element (due to navigations used on aggregate chain), doesn't translate to aggregate anymore since we need to translate the join on parent query, remove the translated join if the chain didn't end in aggregate and also de-dupe same joins. Filing issue to improve this in future. Due to fragile nature of matching to lift the join, we shouldn't try to lift joins. - To support custom aggregate operations, we will either reused `IMethodCallTranslator` or create a parallel structure for aggregate methods and call into it from SqlTranslator by passing translated SqlEnumerableExpression as appropriate. - For complex grouping key, we cause a pushdown so that we can reference the grouping key through columns only. This allows us to reference the grouping key in correlation predicate for subquery without generating invalid SQL in many cases. - With complex grouping key converting to columns, now we are able to correctly generate identifiers for grouping queries which makes more queries with correlated collections (where either parent or inner both queries can be groupby query) translatable. - Erase client projection when applying aggregate operation over GroupBy result. - When processing result selector in GroupBy use the updated key selector if the select expression was pushed down. Resolves #27132 Resolves #27266 Resolves #27433 Resolves #23601 Resolves #27721 Resolves #27796 Resolves #27801 Resolves #19683 Relates to #22957
smitpatel
added a commit
that referenced
this issue
May 3, 2022
Design: - Introduce `SqlEnumerableExpression` - a holder class which indicates the `SqlExpression` is in form of a enumerable (or group) coming as a result of whole table selection or a grouping element. It also stores details about if `Distinct` is applied over grouping or if there are any orderings. - Due to above `DistinctExpression` has been removed. The token while used to denote `Distinct` over grouping element were not valid in other parts of SQL tree hence it makes more sense to combine it with `SqlEnumerableExpression`. - To support dual pass, `GroupByShaperExpression` contains 2 forms of grouping element. One element selector form which correlates directly with the parent grouped query, second subquery form which correlates to parent grouped query through a correlation predicate. Element selector is first used to translate aggregation. If that fails we use subquery form to translate as a subquery. Due to 2 forms of same component, GroupByShaperExpression disallows calling into VisitChildren method, any visitor which is visiting a tree containing GroupByShaperExpression (which appears only in `QueryExpression.ShaperExpression` or LINQ expression after remapping but before translation) must intercept the tree and either ignore or process it appropriately. - An internal visitor (`GroupByAggregateChainProcessor`) inside SqlTranslator visits and process chain of queryable operations on a grouping element before aggregate is called and condense it into `SqlEnumerableExpression` which is then passed to method which translates aggregate. This visitor only processes Where/Distinct/Select for now. Future PR will add processing for OrderBy/ThenBy(Descending) operations to generate orderings. - Side-effect above is that joins expanded over the grouping element (due to navigations used on aggregate chain), doesn't translate to aggregate anymore since we need to translate the join on parent query, remove the translated join if the chain didn't end in aggregate and also de-dupe same joins. Filing issue to improve this in future. Due to fragile nature of matching to lift the join, we shouldn't try to lift joins. - To support custom aggregate operations, we will either reused `IMethodCallTranslator` or create a parallel structure for aggregate methods and call into it from SqlTranslator by passing translated SqlEnumerableExpression as appropriate. - For complex grouping key, we cause a pushdown so that we can reference the grouping key through columns only. This allows us to reference the grouping key in correlation predicate for subquery without generating invalid SQL in many cases. - With complex grouping key converting to columns, now we are able to correctly generate identifiers for grouping queries which makes more queries with correlated collections (where either parent or inner both queries can be groupby query) translatable. - Erase client projection when applying aggregate operation over GroupBy result. - When processing result selector in GroupBy use the updated key selector if the select expression was pushed down. Resolves #27132 Resolves #27266 Resolves #27433 Resolves #23601 Resolves #27721 Resolves #27796 Resolves #27801 Resolves #19683 Relates to #22957
smitpatel
added a commit
that referenced
this issue
May 3, 2022
Design: - Introduce `SqlEnumerableExpression` - a holder class which indicates the `SqlExpression` is in form of a enumerable (or group) coming as a result of whole table selection or a grouping element. It also stores details about if `Distinct` is applied over grouping or if there are any orderings. - Due to above `DistinctExpression` has been removed. The token while used to denote `Distinct` over grouping element were not valid in other parts of SQL tree hence it makes more sense to combine it with `SqlEnumerableExpression`. - To support dual pass, `GroupByShaperExpression` contains 2 forms of grouping element. One element selector form which correlates directly with the parent grouped query, second subquery form which correlates to parent grouped query through a correlation predicate. Element selector is first used to translate aggregation. If that fails we use subquery form to translate as a subquery. Due to 2 forms of same component, GroupByShaperExpression disallows calling into VisitChildren method, any visitor which is visiting a tree containing GroupByShaperExpression (which appears only in `QueryExpression.ShaperExpression` or LINQ expression after remapping but before translation) must intercept the tree and either ignore or process it appropriately. - An internal visitor (`GroupByAggregateChainProcessor`) inside SqlTranslator visits and process chain of queryable operations on a grouping element before aggregate is called and condense it into `SqlEnumerableExpression` which is then passed to method which translates aggregate. This visitor only processes Where/Distinct/Select for now. Future PR will add processing for OrderBy/ThenBy(Descending) operations to generate orderings. - Side-effect above is that joins expanded over the grouping element (due to navigations used on aggregate chain), doesn't translate to aggregate anymore since we need to translate the join on parent query, remove the translated join if the chain didn't end in aggregate and also de-dupe same joins. Filing issue to improve this in future. Due to fragile nature of matching to lift the join, we shouldn't try to lift joins. - To support custom aggregate operations, we will either reused `IMethodCallTranslator` or create a parallel structure for aggregate methods and call into it from SqlTranslator by passing translated SqlEnumerableExpression as appropriate. - For complex grouping key, we cause a pushdown so that we can reference the grouping key through columns only. This allows us to reference the grouping key in correlation predicate for subquery without generating invalid SQL in many cases. - With complex grouping key converting to columns, now we are able to correctly generate identifiers for grouping queries which makes more queries with correlated collections (where either parent or inner both queries can be groupby query) translatable. - Erase client projection when applying aggregate operation over GroupBy result. - When processing result selector in GroupBy use the updated key selector if the select expression was pushed down. Resolves #27132 Resolves #27266 Resolves #27433 Resolves #23601 Resolves #27721 Resolves #27796 Resolves #27801 Resolves #19683 Relates to #22957
smitpatel
added a commit
that referenced
this issue
May 4, 2022
Design: - Introduce `SqlEnumerableExpression` - a holder class which indicates the `SqlExpression` is in form of a enumerable (or group) coming as a result of whole table selection or a grouping element. It also stores details about if `Distinct` is applied over grouping or if there are any orderings. - Due to above `DistinctExpression` has been removed. The token while used to denote `Distinct` over grouping element were not valid in other parts of SQL tree hence it makes more sense to combine it with `SqlEnumerableExpression`. - To support dual pass, `GroupByShaperExpression` contains 2 forms of grouping element. One element selector form which correlates directly with the parent grouped query, second subquery form which correlates to parent grouped query through a correlation predicate. Element selector is first used to translate aggregation. If that fails we use subquery form to translate as a subquery. Due to 2 forms of same component, GroupByShaperExpression disallows calling into VisitChildren method, any visitor which is visiting a tree containing GroupByShaperExpression (which appears only in `QueryExpression.ShaperExpression` or LINQ expression after remapping but before translation) must intercept the tree and either ignore or process it appropriately. - An internal visitor (`GroupByAggregateChainProcessor`) inside SqlTranslator visits and process chain of queryable operations on a grouping element before aggregate is called and condense it into `SqlEnumerableExpression` which is then passed to method which translates aggregate. This visitor only processes Where/Distinct/Select for now. Future PR will add processing for OrderBy/ThenBy(Descending) operations to generate orderings. - Side-effect above is that joins expanded over the grouping element (due to navigations used on aggregate chain), doesn't translate to aggregate anymore since we need to translate the join on parent query, remove the translated join if the chain didn't end in aggregate and also de-dupe same joins. Filing issue to improve this in future. Due to fragile nature of matching to lift the join, we shouldn't try to lift joins. - To support custom aggregate operations, we will either reused `IMethodCallTranslator` or create a parallel structure for aggregate methods and call into it from SqlTranslator by passing translated SqlEnumerableExpression as appropriate. - For complex grouping key, we cause a pushdown so that we can reference the grouping key through columns only. This allows us to reference the grouping key in correlation predicate for subquery without generating invalid SQL in many cases. - With complex grouping key converting to columns, now we are able to correctly generate identifiers for grouping queries which makes more queries with correlated collections (where either parent or inner both queries can be groupby query) translatable. - Erase client projection when applying aggregate operation over GroupBy result. - When processing result selector in GroupBy use the updated key selector if the select expression was pushed down. Resolves #27132 Resolves #27266 Resolves #27433 Resolves #23601 Resolves #27721 Resolves #27796 Resolves #27801 Resolves #19683 Relates to #22957
smitpatel
added a commit
that referenced
this issue
May 4, 2022
…7931) Design: - Introduce `SqlEnumerableExpression` - a holder class which indicates the `SqlExpression` is in form of a enumerable (or group) coming as a result of whole table selection or a grouping element. It also stores details about if `Distinct` is applied over grouping or if there are any orderings. - Due to above `DistinctExpression` has been removed. The token while used to denote `Distinct` over grouping element were not valid in other parts of SQL tree hence it makes more sense to combine it with `SqlEnumerableExpression`. - To support dual pass, `GroupByShaperExpression` contains 2 forms of grouping element. One element selector form which correlates directly with the parent grouped query, second subquery form which correlates to parent grouped query through a correlation predicate. Element selector is first used to translate aggregation. If that fails we use subquery form to translate as a subquery. Due to 2 forms of same component, GroupByShaperExpression disallows calling into VisitChildren method, any visitor which is visiting a tree containing GroupByShaperExpression (which appears only in `QueryExpression.ShaperExpression` or LINQ expression after remapping but before translation) must intercept the tree and either ignore or process it appropriately. - An internal visitor (`GroupByAggregateChainProcessor`) inside SqlTranslator visits and process chain of queryable operations on a grouping element before aggregate is called and condense it into `SqlEnumerableExpression` which is then passed to method which translates aggregate. This visitor only processes Where/Distinct/Select for now. Future PR will add processing for OrderBy/ThenBy(Descending) operations to generate orderings. - Side-effect above is that joins expanded over the grouping element (due to navigations used on aggregate chain), doesn't translate to aggregate anymore since we need to translate the join on parent query, remove the translated join if the chain didn't end in aggregate and also de-dupe same joins. Filing issue to improve this in future. Due to fragile nature of matching to lift the join, we shouldn't try to lift joins. - To support custom aggregate operations, we will either reused `IMethodCallTranslator` or create a parallel structure for aggregate methods and call into it from SqlTranslator by passing translated SqlEnumerableExpression as appropriate. - For complex grouping key, we cause a pushdown so that we can reference the grouping key through columns only. This allows us to reference the grouping key in correlation predicate for subquery without generating invalid SQL in many cases. - With complex grouping key converting to columns, now we are able to correctly generate identifiers for grouping queries which makes more queries with correlated collections (where either parent or inner both queries can be groupby query) translatable. - Erase client projection when applying aggregate operation over GroupBy result. - When processing result selector in GroupBy use the updated key selector if the select expression was pushed down. Resolves #27132 Resolves #27266 Resolves #27433 Resolves #23601 Resolves #27721 Resolves #27796 Resolves #27801 Resolves #19683 Relates to #22957
smitpatel
added a commit
that referenced
this issue
May 24, 2022
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
ghost
closed this as
ghost
pushed a commit
that referenced
this issue
May 25, 2022
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
|
@smitpatel keeping open just in order to verify top-level custom aggregate support (as in #28092 (comment)), which may require some extra work. |
|
Filed #28097 for additional work. |
ajcvickers
added
the
closed-fixed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
and others
Following discussion in #13278, I've done some exploratory work in npgsql/efcore.pg#1531 on PostgreSQL aggregate functions. Here are some issues I ran into:
/cc @smitpatel
Interesting candidate translations:
The text was updated successfully, but these errors were encountered: