kgyrtkirk opened a new pull request, #15045: URL: https://github.com/apache/druid/pull/15045
Most of the testcases were disabled in `CalciteWindowQueryTest` during the Calcite-1.35 upgrade; there were some changes arising from the fact that the removal of `DRUID_SUM` had some unexpected sideffects: * `SqlStdOperatorTable.SUM` became the `SUM` operator * because of that `SqlToRelConverter` started rewriting windowed `SUM` -s into `SUM0` -s * my opinion is that w.r.t to Druid this rewrite provides no real advantage - as `SUM0` is serviced by `SUM` [here](https://github.com/apache/druid/blob/8e22a178cc9925fd28ff3bd738fa01f0888eea7c/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/SumZeroSqlAggregator.java#L25C43-L25C59) * I believe that's not 100% correct in cases when it aggregates just `null`-s but that doesnt matter in this case I propose to introduce back a local `DRUID_SUM` thing as an unchanged `SUM` and later when CALCITE-6020 is fixed ; we can drop that. This PR has: - [x] been self-reviewed. - [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
