xuzifu666 commented on code in PR #5339:
URL: https://github.com/apache/calcite/pull/5339#discussion_r4326030609
##########
core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java:
##########
@@ -395,6 +395,16 @@ public void send(Row row) {
}
public void end(Sink sink) throws InterruptedException {
+ if (accumulators.isEmpty() && grouping.isEmpty()) {
Review Comment:
Agreed and I wouldn't want to invest much in the interpreter either.
The fix is only a few lines in `AggregateNode.Grouping.end`, and it keeps
the two runtimes consistent now that `PruneEmptyRules` no longer prunes an
aggregate with an empty grouping set: without it, the interpreter would still
return 0 rows while the enumerable path returns the grand-total row. I've also
dropped the stale issue reference from the comment there.
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableAggregate.java:
##########
@@ -280,20 +287,48 @@ public EnumerableAggregate(RelOptCluster cluster,
RelTraitSet traitSet,
resultBlock.toBlock(),
requireNonNull(key_, "key_"),
acc_));
+ final Expressions.FluentList<Expression> arguments =
+ Expressions.list(childExp,
+ keySelectors_,
+ Expressions.call(lambdaFactory,
+ BuiltInMethod.AGG_LAMBDA_FACTORY_ACC_INITIALIZER.method),
+ Expressions.call(lambdaFactory,
+ BuiltInMethod.AGG_LAMBDA_FACTORY_ACC_ADDER.method),
+ Expressions.call(lambdaFactory,
+ BuiltInMethod.AGG_LAMBDA_FACTORY_ACC_RESULT_SELECTOR.method,
+ resultSelector));
+ final Method groupByMultipleMethod;
+ if (hasEmptyGroup) {
+ // The key of the empty grouping set is a constant: each grouping
+ // column is defaulted (null, or the default value of its primitive
+ // type) and each indicator is true. It is built directly, mirroring
+ // the selector that generateSelector builds for that grouping set,
+ // so that groupByMultiple can make the empty grouping set produce
+ // its row when the input is empty, as GROUP BY () does (see
+ // CALCITE-1059).
+ final List<Expression> keyExpressions = new ArrayList<>();
+ for (int j = 0; j < groupCount; j++) {
+ final Primitive primitive =
+ Primitive.of(keyPhysType.fieldClass(j));
+ keyExpressions.add(
+ Expressions.constant(
+ primitive != null ? primitive.defaultValue : null));
+ }
+ for (int j = 0; j < groupCount; j++) {
+ keyExpressions.add(Expressions.constant(true));
+ }
+ final Expression emptyGroupKey =
+ builder.append("emptyGroupKey",
+ keyPhysType.record(keyExpressions));
+ arguments.add(emptyGroupKey);
+ groupByMultipleMethod = BuiltInMethod.GROUP_BY_MULTIPLE2.method;
Review Comment:
Done.
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableAggregate.java:
##########
@@ -280,20 +287,48 @@ public EnumerableAggregate(RelOptCluster cluster,
RelTraitSet traitSet,
resultBlock.toBlock(),
requireNonNull(key_, "key_"),
acc_));
+ final Expressions.FluentList<Expression> arguments =
+ Expressions.list(childExp,
+ keySelectors_,
+ Expressions.call(lambdaFactory,
+ BuiltInMethod.AGG_LAMBDA_FACTORY_ACC_INITIALIZER.method),
+ Expressions.call(lambdaFactory,
+ BuiltInMethod.AGG_LAMBDA_FACTORY_ACC_ADDER.method),
+ Expressions.call(lambdaFactory,
+ BuiltInMethod.AGG_LAMBDA_FACTORY_ACC_RESULT_SELECTOR.method,
+ resultSelector));
+ final Method groupByMultipleMethod;
+ if (hasEmptyGroup) {
+ // The key of the empty grouping set is a constant: each grouping
Review Comment:
Done, I had added comments to explain the behavior on its own: the key of
the empty grouping set is a constant, built the same way as the key the
selector for that grouping set computes for any input row, and passed to
groupByMultiple so that the empty grouping set produces its single row when the
input is empty, just as a query with no GROUP BY produces a row over an empty
input.
--
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]
