[ 
https://issues.apache.org/jira/browse/CALCITE-2051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16257582#comment-16257582
 ] 

Julian Hyde commented on CALCITE-2051:
--------------------------------------

I think it was a mistake for Calcite to allow such relational expressions. 
Ideally it should reject an Aggregate if there are columns that are not in any 
of the grouping sets:

{noformat}
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java 
b/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java
index 93f9948..d76c504 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java
@@ -156,6 +156,9 @@ protected Aggregate(
         assert groupSet.contains(set);
       }
     }
+    Preconditions.checkArgument(
+        groupSet.equals(ImmutableBitSet.union(this.groupSets)),
+        "groupSet must be minimal", groupSet, groupSets);
     assert groupSet.length() <= child.getRowType().getFieldCount();
     for (AggregateCall aggCall : aggCalls) {
       assert typeMatchesInferred(aggCall, Litmus.THROW);
{noformat}

(I was assuming that when I defined SIMPLE, I just forgot to check the 
pre-condition.) If we were to make that change, can we change Hive to respect 
it?

> Rules using Aggregate might check for simple grouping sets incorrectly
> ----------------------------------------------------------------------
>
>                 Key: CALCITE-2051
>                 URL: https://issues.apache.org/jira/browse/CALCITE-2051
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>            Reporter: Jesus Camacho Rodriguez
>            Assignee: Jesus Camacho Rodriguez
>            Priority: Critical
>             Fix For: 1.15.0
>
>
> CALCITE-1069 removed the indicator columns for Aggregate operators. In some 
> places, the indicator boolean check was replaced by the following check: 
> {{aggregate.getGroupSets().size() > 1}}. However, that check is incomplete, 
> it should have been replaced by {{aggregate.getGroupType() != Group.SIMPLE}}.
> For instance : 
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectMergeRule.java#L91



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to