somandal commented on code in PR #10846:
URL: https://github.com/apache/pinot/pull/10846#discussion_r1219834738


##########
pinot-query-planner/src/main/java/org/apache/calcite/sql/fun/PinotOperatorTable.java:
##########
@@ -103,6 +98,61 @@ public final void initNoDuplicate() {
         throw Util.throwAsRuntime(Util.causeOrSelf(e));
       }
     }
+
+    // Walk through all the Pinot aggregation types and register those that 
are supported in multistage and which
+    // aren't standard Calcite functions such as SUM / MIN / MAX / COUNT etc.
+    for (AggregationFunctionType aggregationFunctionType : 
AggregationFunctionType.values()) {
+      if (aggregationFunctionType.isNativeCalciteAggregationFunctionType()
+          || !aggregationFunctionType.isSupportedInMultiStage()) {

Review Comment:
   Are you talking about `isSupportedInMultiStage()`? I can perhaps rename it 
to something like `registerWithOperatorTable()` or something like that for now.
   
   This method is meant to be a temporary one which we will remove once support 
for all aggregation functions is added to multistage. I don't want to keep a 
hard-coded list here because eventually most aggregation functions will be 
supported and the full list is available as part of the 
`AggregationFunctionType` enum class anyways.
   
   Also note that there are many MV aggregation functions which we cannot 
support yet without generically adding MV support to multi-stage as part of 
this OSS issue: https://github.com/apache/pinot/issues/10658 
   So we may need to gatekeep which aggregation functions are supported for a 
while, but over time it should be possible to support most.
   
   I'm not sure what you mean by backward incompatibility when we make v2 the 
default? Can you elaborate a bit on that part?



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to