weidong3630 commented on a change in pull request #1606: [CALCITE-3466] Do not
drop the "group by" clause of subquery in a select statement with "group by"
clause in JDBC adapter.
URL: https://github.com/apache/calcite/pull/1606#discussion_r365804262
##########
File path:
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
##########
@@ -1224,10 +1224,18 @@ public Builder builder(RelNode rel, Clause... clauses)
{
needNew = true;
}
}
- if (rel instanceof Aggregate
- && !dialect.supportsNestedAggregations()
- && hasNestedAggregations((Aggregate) rel)) {
- needNew = true;
+
+ if (rel instanceof Aggregate) {
+ Aggregate e = (Aggregate) rel;
+ final boolean nestedAgg = hasNestedAggregations(e);
+ if (withGroupBy() && (!e.getGroupSet().toList().isEmpty()
+ || !nestedAgg || !dialect.supportsNestedAggregations())) {
Review comment:
> -- Update
> Sorry, @julianhyde the "public" there is a typo and my intention was to
make the `canApplyNestedAggregation` **private** , I update the method as below
>
> ```
> private boolean canApplyNestedAggregation(Aggregate agg) {
> return agg.getGroupCount() == 0
> && hasNestedAggregations(agg)
> && dialect.supportsNestedAggregations();
> }
> ```
>
> I also think line 1231 and 1232 are very confusing. That's why I propose
to use `canApplyNestedAggregation` instead.
> We can apply two `Aggregate`s onto a single `SqlSelect` only when it
satisfies `canApplyNestedAggregation`, which checks if the outer `agg` is
"!isNotGrandTotal"
Hi, like that `!(A and B and C)` does not equal to `!A or !B or !C`,
`!canApplyNestedAggregation` does not equal to
`(!e.getGroupSet().toList().isEmpty()
|| !nestedAgg || !dialect.supportsNestedAggregations())`, is
that right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
