mihaibudiu commented on code in PR #5339:
URL: https://github.com/apache/calcite/pull/5339#discussion_r4318980889
##########
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:
it would be much better to explain what happens here without references to
calcite issues closed long ago.
##########
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:
it would help a lot the reviewers if you showed in a comment the generated
Java code.
##########
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:
I think the interpreter is hopelessly unmaintained, but it's fine to fix
bugs here and there.
--
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]
