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

Stamatis Zampetakis commented on CALCITE-6020:
----------------------------------------------

The code would be more modular if the SqlToRelConverter didn't perform directly 
optimizations/simplifications since that gives more flexibility to the users. I 
like the approach of having this in other places such as rules (e.g., 
[AggregateReduceFunctionsRule|https://github.com/apache/calcite/blob/aef9bdb5c091e4df84eeb47a9aa70770dd3d3fb8/core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java]),
 convertlets (e.g., 
[SqlRexConvertletTable|https://github.com/apache/calcite/blob/aef9bdb5c091e4df84eeb47a9aa70770dd3d3fb8/core/src/main/java/org/apache/calcite/sql2rel/SqlRexConvertletTable.java]),
 etc. Maybe we could extract this reduction to another place as well.

SUM vs SUM0 has been a debatable topic lately so I am in favor of options that 
make this decision configurable/modular.

> SqlToRelConverter should not replace windowed SUM with equivalent expression 
> using SUM0
> ---------------------------------------------------------------------------------------
>
>                 Key: CALCITE-6020
>                 URL: https://issues.apache.org/jira/browse/CALCITE-6020
>             Project: Calcite
>          Issue Type: Improvement
>            Reporter: Zoltan Haindrich
>            Assignee: Zoltan Haindrich
>            Priority: Major
>              Labels: pull-request-available
>
> {{SqlToRelConverter}} replaces {{SUM}} with {{SUM0}} around 
> [here|https://github.com/apache/calcite/blob/e1991e08a225ef08c2402ab35c310d88fff3c222/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5885]
> This might have been needed at some point in the past - but I think it will 
> be better to leave it as {{SUM}} - as in case there is no {{SUM0}} in the 
> system that will be replaced with a {{COALESCE(SUM(...) , 0 )}} to provide it 
> - as see 
> [here|https://github.com/apache/calcite/blob/e1991e08a225ef08c2402ab35c310d88fff3c222/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java#L1288]



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

Reply via email to