[ 
https://issues.apache.org/jira/browse/CALCITE-6492?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17868048#comment-17868048
 ] 

Zoltan Haindrich commented on CALCITE-6492:
-------------------------------------------

I might have missed the distinction between the two because 
`getDistinctOptionality` was used in 
[AggregateExpandDistinctAggregatesRule|https://github.com/apache/calcite/blob/ea1a255fd071a518fe8d30e361efa0696164a037/core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java#L826]
 as well.
Thank you for the pointers.

I feel like adding properties like these should follow is the interface way 
{{singleton}} and {{splittable}} was using - and use it with {{instanceof`}} or 
introduce either an {{as}} method or some Util functions.
I don't feel like it should be more confined than just an interface; as pieces 
which could do more like rewrites/etc could benefit from the freedom of having 
an interface more.

However in this case I feel like it would make sense to add it to the main 
interface; as `distinct` is part of the `SqlAggFunction`'s API.
I believe the best default answer for the question {{isNativelyExecutable}} by 
the current engine would be to `return !distinct;` - so that implementors have 
to override it to show that its allowed.
It would be nice to also check this for the produced plan - I'll see if that's 
possible

> Support aggregate functions which could process DISTINCT natively
> -----------------------------------------------------------------
>
>                 Key: CALCITE-6492
>                 URL: https://issues.apache.org/jira/browse/CALCITE-6492
>             Project: Calcite
>          Issue Type: Improvement
>            Reporter: Zoltan Haindrich
>            Assignee: Zoltan Haindrich
>            Priority: Major
>
> This could be usefull if the execution engine natively supports some distinct 
> aggregations natively - there is no rewrite necessary for these functions.
> Currently there is support 
> [SqlAggFunction#getDistinctOptionality|https://github.com/apache/calcite/blob/0deab6f7e0cb4ec63eae8b59477d6f0fadfd11e8/core/src/main/java/org/apache/calcite/sql/SqlAggFunction.java#L187-L189]
>  - which have overlaps with this - possibly the closest would be to set it to 
> *IGNORED* if its supported natively...however
> * that's a bit misleading as its not IGNORED; but supported...
> * there is also 
> [checkArgument|https://github.com/apache/calcite/blob/0deab6f7e0cb4ec63eae8b59477d6f0fadfd11e8/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java#L125]
>  which ensures that *distinct* is not accepted in tht case.
> More or less the end result would be to also enhance 
> AggregateExpandDistinctAggregatesRule with the ability to ignore aggregates.
> note: In Druid
> * if approximationCountDistinct is disabled ; that [enables a calcite rule 
> which rewrites *all* disitnct 
> aggregates|https://github.com/apache/druid/blob/master/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java#L496-L503]
> * in the meantime there are also some aggregate functions which support 
> *distinct* natively like 
> [string_agg|https://github.com/apache/druid/blob/c9aae9d8e683c0cc9c4687e526b8270f744c57c2/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/StringSqlAggregator.java#L154]
>  - which doesn't need any rewrites



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to