vvivekiyer commented on code in PR #10248:
URL: https://github.com/apache/pinot/pull/10248#discussion_r1116420746


##########
pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotAggregateExchangeNodeInsertRule.java:
##########
@@ -169,36 +183,136 @@ private RelNode makeNewIntermediateAgg(RelOptRuleCall 
ruleCall, Aggregate oldAgg
    */
   private static void convertAggCall(RexBuilder rexBuilder, Aggregate 
oldAggRel, int oldCallIndex,
       AggregateCall oldCall, List<AggregateCall> newCalls, Map<AggregateCall, 
RexNode> aggCallMapping,
-      List<RexNode> inputExprs) {
+      boolean isLeafStageAggregationPresent, List<Integer> argList) {
     final int nGroups = oldAggRel.getGroupCount();
     final SqlAggFunction oldAggregation = oldCall.getAggregation();
     final SqlKind aggKind = oldAggregation.getKind();
     // Check only the supported AGG functions are provided.
-    Preconditions.checkState(SUPPORTED_AGG_KIND.contains(aggKind), 
"Unsupported SQL aggregation "
-        + "kind: {}. Only splittable aggregation functions are supported!", 
aggKind);
+    Preconditions.checkState(SUPPORTED_AGG_KIND.contains(aggKind),
+        "Unsupported SQL aggregation " + "kind: {}. Only splittable 
aggregation functions are supported!", aggKind);
 
-    // Special treatment on COUNT
     AggregateCall newCall;
-    if (oldAggregation instanceof SqlCountAggFunction) {
-      newCall = AggregateCall.create(new SqlSumEmptyIsZeroAggFunction(), 
oldCall.isDistinct(), oldCall.isApproximate(),
-          oldCall.ignoreNulls(), convertArgList(nGroups + oldCallIndex, 
Collections.singletonList(oldCallIndex)),
-          oldCall.filterArg, oldCall.distinctKeys, oldCall.collation, 
oldCall.type, oldCall.getName());
+    if (isLeafStageAggregationPresent) {
+      // Special treatment for Count. If count is performed at the Leaf Stage, 
a Sum needs to be performed at the
+      // intermediate stage.
+      if (oldAggregation instanceof SqlCountAggFunction) {
+        newCall =
+            AggregateCall.create(new SqlSumEmptyIsZeroAggFunction(), 
oldCall.isDistinct(), oldCall.isApproximate(),
+                oldCall.ignoreNulls(), convertArgList(nGroups + oldCallIndex, 
Collections.singletonList(oldCallIndex)),
+                oldCall.filterArg, oldCall.distinctKeys, oldCall.collation, 
oldCall.type, oldCall.getName());
+      } else {
+        newCall = AggregateCall.create(oldCall.getAggregation(), 
oldCall.isDistinct(), oldCall.isApproximate(),
+            oldCall.ignoreNulls(), convertArgList(nGroups + oldCallIndex, 
oldCall.getArgList()), oldCall.filterArg,
+            oldCall.distinctKeys, oldCall.collation, oldCall.type, 
oldCall.getName());
+      }
     } else {
-      newCall = AggregateCall.create(
-          oldCall.getAggregation(), oldCall.isDistinct(), 
oldCall.isApproximate(), oldCall.ignoreNulls(),
-          convertArgList(nGroups + oldCallIndex, oldCall.getArgList()), 
oldCall.filterArg, oldCall.distinctKeys,
-          oldCall.collation, oldCall.type, oldCall.getName());
+      List<Integer> newArgList = oldCall.getArgList().size() == 0 ? 
Collections.emptyList()
+          : Collections.singletonList(argList.get(oldCallIndex));
+
+      newCall = AggregateCall.create(oldCall.getAggregation(), 
oldCall.isDistinct(), oldCall.isApproximate(),
+          oldCall.ignoreNulls(), newArgList, oldCall.filterArg, 
oldCall.distinctKeys, oldCall.collation, oldCall.type,
+          oldCall.getName());
     }
-    rexBuilder.addAggCall(newCall,
-        nGroups,
-        newCalls,
-        aggCallMapping,
-        oldAggRel.getInput()::fieldIsNullable);
+
+    rexBuilder.addAggCall(newCall, nGroups, newCalls, aggCallMapping, 
oldAggRel.getInput()::fieldIsNullable);
   }
 
   private static List<Integer> convertArgList(int oldCallIndexWithShift, 
List<Integer> argList) {
     Preconditions.checkArgument(argList.size() <= 1,
         "Unable to convert call as the argList contains more than 1 argument");
     return argList.size() == 1 ? 
Collections.singletonList(oldCallIndexWithShift) : Collections.emptyList();
   }
+
+  private void createPlanWithoutLeafAggregation(RelOptRuleCall call) {
+    Aggregate oldAggRel = call.rel(0);
+    RelNode childRel = ((HepRelVertex) oldAggRel.getInput()).getCurrentRel();
+    LogicalProject project;
+
+    List<Integer> newAggArgColumns = new ArrayList<>();
+    List<Integer> newAggGroupByColumns = new ArrayList<>();
+
+    // 1. Create the LogicalProject node if it does not exist. This is to send 
only the relevant columns over
+    //    the wire for intermediate aggregation.
+    if (childRel instanceof Project) {
+      // Avoid creating a new LogicalProject if the child node of aggregation 
is already a project node.

Review Comment:
   No. Calcite project makes sure to project the column only once. I added a 
test case for this.



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