[GitHub] [ignite-3] korlov42 commented on a diff in pull request #2312: [IGNITE-19785] Sql. Rework execution of 2-phase aggregates

["korlov42 (via GitHub)" <gi...@apache.org>]

korlov42 commented on code in PR #2312:
URL: https://github.com/apache/ignite-3/pull/2312#discussion_r1279358135


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/HashAggregateNode.java:
##########
@@ -51,10 +51,14 @@ public class HashAggregateNode<RowT> extends AbstractNode<RowT> implements Singl
 
     private final RowFactory<RowT> rowFactory;
 
-    private final ImmutableBitSet grpSet;
+    /** A bit set that contains fields included in all grouping sets. */
+    private final ImmutableBitSet allFields;
 
     private final List<Grouping> groupings;
 
+    /** Mapping between group columns and their positions inside an input row.*/
+    private final Mapping mapping;

Review Comment:
   may you elaborate, why do we need this mapping?



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/PlanUtils.java:
##########
@@ -46,4 +60,126 @@ public static boolean complexDistinctAgg(List<AggregateCall> aggCalls) {
         }
         return false;
     }
+
+    /**
+     * Creates a row type for REDUCE phase of a two phase aggregate used by hash-based map/reduce implementation.

Review Comment:
   beside, javadoc state that this method creates row type for REDUCE phase, but the only usage is for `IgniteMapSortAggregate`



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/PlanUtils.java:
##########
@@ -46,4 +60,126 @@ public static boolean complexDistinctAgg(List<AggregateCall> aggCalls) {
         }
         return false;
     }
+
+    /**
+     * Creates a row type for REDUCE phase of a two phase aggregate used by hash-based map/reduce implementation.

Review Comment:
   did you mean `sort-based`?



-- 
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: notifications-unsubscribe@ignite.apache.org

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