yashmayya commented on PR #17109: URL: https://github.com/apache/pinot/pull/17109#issuecomment-3564485153
> Replacing SqlStdOperatorTable.SUM and SqlStdOperatorTable.AVG with our custom PinotSqlAggFunction instances looks to be causing a number of issues with Calcite optimizations that rely on the exact standard operator instances. For example - [window AVG reduce](https://github.com/apache/calcite/blob/34989b0ed7793cedf713c2f159de6247a730458c/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java#L346-L347), or [window SUM null when count zero](https://github.com/apache/calcite/blob/34989b0ed7793cedf713c2f159de6247a730458c/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L6287-L6301). Some of these are pluggable, whereas others don't seem to be. I'm wondering whether it would be more prudent to require users to use ARRAY_TO_MV with standard aggregation functions on MV types instead. I've gone back to using custom operators so that `ARRAY_TO_MV` is _not_ required. I've also made some changes like introducing native support for `AVG` window function, and using a custom SUM `SqlAggFunction` that retains splittability (https://github.com/apache/calcite/blob/49fa3edbb98e3f01fccc028c517e8abf78c1f49f/core/src/main/java/org/apache/calcite/rel/rules/AggregateRemoveRule.java#L89-L100). -- 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]
