xiedeyantu commented on code in PR #4371:
URL: https://github.com/apache/calcite/pull/4371#discussion_r2098013761
##########
core/src/main/java/org/apache/calcite/rel/rules/AggregateValuesRule.java:
##########
@@ -72,40 +89,54 @@ public AggregateValuesRule(RelBuilderFactory
relBuilderFactory) {
.as(Config.class));
}
+ //~ Methods ----------------------------------------------------------------
+
@Override public void onMatch(RelOptRuleCall call) {
final Aggregate aggregate = call.rel(0);
final Values values = call.rel(1);
Util.discard(values);
final RelBuilder relBuilder = call.builder();
final RexBuilder rexBuilder = relBuilder.getRexBuilder();
- final List<RexLiteral> literals = new ArrayList<>();
- for (final AggregateCall aggregateCall : aggregate.getAggCallList()) {
- switch (aggregateCall.getAggregation().getKind()) {
- case COUNT:
- case SUM0:
- literals.add(
- rexBuilder.makeLiteral(BigDecimal.ZERO, aggregateCall.getType()));
- break;
-
- case MIN:
- case MAX:
- case SUM:
- literals.add(rexBuilder.makeNullLiteral(aggregateCall.getType()));
- break;
-
- default:
- // Unknown what this aggregate call should do on empty Values. Bail
out to be safe.
- return;
+ if (aggregate.getGroupCount() == 0 && values.getTuples().isEmpty()) {
+ final List<RexLiteral> literals = new ArrayList<>();
+ for (final AggregateCall aggregateCall : aggregate.getAggCallList()) {
+ switch (aggregateCall.getAggregation().getKind()) {
+ case COUNT:
+ case SUM0:
+ literals.add(
+ rexBuilder.makeLiteral(BigDecimal.ZERO,
aggregateCall.getType()));
+ break;
+
+ case MIN:
+ case MAX:
+ case SUM:
+ literals.add(rexBuilder.makeNullLiteral(aggregateCall.getType()));
+ break;
+
+ default:
+ // Unknown what this aggregate call should do on empty Values. Bail
out to be safe.
+ return;
+ }
}
- }
-
- call.transformTo(
- relBuilder.values(ImmutableList.of(literals), aggregate.getRowType())
- .build());
- // New plan is absolutely better than old plan.
- call.getPlanner().prune(aggregate);
+ call.transformTo(
+ relBuilder.values(ImmutableList.of(literals), aggregate.getRowType())
+ .build());
+
+ // New plan is absolutely better than old plan.
+ call.getPlanner().prune(aggregate);
+ } else {
+ if (Aggregate.isSimple(aggregate)
+ && aggregate.getAggCallList().isEmpty()
+ && aggregate.getRowType().equals(values.getRowType())) {
+ List<ImmutableList<RexLiteral>> distinctValues =
+
values.getTuples().stream().distinct().collect(Collectors.toList());
+ relBuilder.values(distinctValues, values.getRowType());
+ call.transformTo(relBuilder.build());
+ call.getPlanner().prune(aggregate);
Review Comment:
Repeated code can be mentioned outside the if else.
##########
core/src/main/java/org/apache/calcite/rel/rules/AggregateValuesRule.java:
##########
@@ -72,40 +89,54 @@ public AggregateValuesRule(RelBuilderFactory
relBuilderFactory) {
.as(Config.class));
}
+ //~ Methods ----------------------------------------------------------------
+
@Override public void onMatch(RelOptRuleCall call) {
final Aggregate aggregate = call.rel(0);
final Values values = call.rel(1);
Util.discard(values);
final RelBuilder relBuilder = call.builder();
final RexBuilder rexBuilder = relBuilder.getRexBuilder();
- final List<RexLiteral> literals = new ArrayList<>();
- for (final AggregateCall aggregateCall : aggregate.getAggCallList()) {
- switch (aggregateCall.getAggregation().getKind()) {
- case COUNT:
- case SUM0:
- literals.add(
- rexBuilder.makeLiteral(BigDecimal.ZERO, aggregateCall.getType()));
- break;
-
- case MIN:
- case MAX:
- case SUM:
- literals.add(rexBuilder.makeNullLiteral(aggregateCall.getType()));
- break;
-
- default:
- // Unknown what this aggregate call should do on empty Values. Bail
out to be safe.
- return;
+ if (aggregate.getGroupCount() == 0 && values.getTuples().isEmpty()) {
+ final List<RexLiteral> literals = new ArrayList<>();
+ for (final AggregateCall aggregateCall : aggregate.getAggCallList()) {
+ switch (aggregateCall.getAggregation().getKind()) {
+ case COUNT:
+ case SUM0:
+ literals.add(
+ rexBuilder.makeLiteral(BigDecimal.ZERO,
aggregateCall.getType()));
+ break;
+
+ case MIN:
+ case MAX:
+ case SUM:
+ literals.add(rexBuilder.makeNullLiteral(aggregateCall.getType()));
+ break;
+
+ default:
+ // Unknown what this aggregate call should do on empty Values. Bail
out to be safe.
+ return;
+ }
}
- }
-
- call.transformTo(
- relBuilder.values(ImmutableList.of(literals), aggregate.getRowType())
- .build());
- // New plan is absolutely better than old plan.
- call.getPlanner().prune(aggregate);
+ call.transformTo(
+ relBuilder.values(ImmutableList.of(literals), aggregate.getRowType())
+ .build());
+
+ // New plan is absolutely better than old plan.
+ call.getPlanner().prune(aggregate);
+ } else {
+ if (Aggregate.isSimple(aggregate)
Review Comment:
Here you can use a separate if, no need for an if else.
--
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]
