lowka commented on code in PR #2443:
URL: https://github.com/apache/ignite-3/pull/2443#discussion_r1298631600
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/AbstractSetOpNode.java:
##########
@@ -195,31 +196,32 @@ private void flush() throws Exception {
}
/**
- * Grouping.
- * TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859
+ * Grouping provides base driver code to implement a set operator.
+ * The basic idea is to store the number of distinct rows per input set
and use these numbers to calculate
+ * the number of rows an operator should produce.
*/
protected abstract static class Grouping<RowT> {
protected final Map<GroupKey, int[]> groups = new HashMap<>();
- protected final RowHandler<RowT> hnd;
-
protected final AggregateType type;
protected final boolean all;
protected final RowFactory<RowT> rowFactory;
- /** Processed rows count in current set. */
- protected int rowsCnt = 0;
+ private final RowHandler<RowT> hnd;
- protected Grouping(ExecutionContext<RowT> ctx, RowFactory<RowT>
rowFactory, AggregateType type, boolean all) {
+ private final int columnNum;
+
+ protected Grouping(ExecutionContext<RowT> ctx, RowFactory<RowT>
rowFactory, int columnNum, AggregateType type, boolean all) {
hnd = ctx.rowHandler();
+ this.columnNum = columnNum;
this.type = type;
this.all = all;
this.rowFactory = rowFactory;
}
- private void add(RowT row, int setIdx) {
+ protected void add(RowT row, int setIdx) {
if (type == AggregateType.REDUCE) {
Review Comment:
Java until recently did not have exhaustive checks `switch` one should
always add `default` clause even if those clauses are unreachable.
I agree that replacing this `if-else` blocks with `switch` statements would
improve the readability of this code a little bit.
--
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]
