ankitsultana commented on code in PR #13561:
URL: https://github.com/apache/pinot/pull/13561#discussion_r1671245205


##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotAggregateExchangeNodeInsertRule.java:
##########
@@ -330,4 +331,15 @@ private static AggregateCall buildAggCall(RelNode input, 
AggregateCall orgAggCal
         ImmutableList.of(), aggType.isInputIntermediateFormat() ? -1 : 
orgAggCall.filterArg, orgAggCall.distinctKeys,
         orgAggCall.collation, numGroups, input, null, null);
   }
+
+  @Nullable
+  private static List<RexNode> findImmediateProjects(RelNode relNode) {
+    relNode = PinotRuleUtils.unboxRel(relNode);
+    if (relNode instanceof Project) {
+      return ((Project) relNode).getProjects();
+    } else if (relNode instanceof Union) {

Review Comment:
   Yeah there could be more. For instance, if we have a Filter or even a Sort, 
we should ideally be able to hoist a literal up to the aggregate node.
   
   I am a bit concerned if that could have edge-cases, that's why I kept the 
scope of the change in this PR small.
   
   (digression follows)
   
   Ideally, I think we should do this step in a separate optimization rule (run 
before agg exchange insert) which is run only for aggregate nodes.
   
   Clubbing this with exchange insertion makes it a bit complicated to reason 
about.



-- 
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]

Reply via email to