danny0405 commented on a change in pull request #1741: [CALCITE-3721] Filter of 
distinct aggregate call is lost after applyi…
URL: https://github.com/apache/calcite/pull/1741#discussion_r366251702
 
 

 ##########
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/AggregateExpandDistinctAggregatesRule.java
 ##########
 @@ -432,31 +442,43 @@ private void rewriteUsingGroupingSets(RelOptRuleCall 
call,
     relBuilder.push(aggregate.getInput());
     final int groupCount = fullGroupSet.cardinality();
 
-    final Map<ImmutableBitSet, Integer> filters = new LinkedHashMap<>();
-    final int z = groupCount + distinctAggCalls.size();
+    // get the base ordinal for filter args for different groupSet.
+    final Map<Pair<ImmutableBitSet, Integer>, Integer> filters = new 
LinkedHashMap<>();
+    int z = groupCount + distinctAggCalls.size();
+    for (ImmutableBitSet groupSet: groupSets) {
+      Set<Integer> filterArgList = distinctFilterArgMap.get(groupSet);
+      for (Integer filterArg: filterArgList) {
+        filters.put(Pair.of(groupSet, filterArg), z);
+        z += 1;
+      }
+    }
+
     distinctAggCalls.add(
         AggregateCall.create(SqlStdOperatorTable.GROUPING, false, false, false,
             ImmutableIntList.copyOf(fullGroupSet), -1, RelCollations.EMPTY,
             groupSets.size(), relBuilder.peek(), null, "$g"));
-    for (Ord<ImmutableBitSet> groupSet : Ord.zip(groupSets)) {
-      filters.put(groupSet.e, z + groupSet.i);
-    }
-
     relBuilder.aggregate(relBuilder.groupKey(fullGroupSet, groupSets),
         distinctAggCalls);
-    final RelNode distinct = relBuilder.peek();
 
     // GROUPING returns an integer (0 or 1). Add a project to convert those
     // values to BOOLEAN.
     if (!filters.isEmpty()) {
       final List<RexNode> nodes = new ArrayList<>(relBuilder.fields());
       final RexNode nodeZ = nodes.remove(nodes.size() - 1);
-      for (Map.Entry<ImmutableBitSet, Integer> entry : filters.entrySet()) {
-        final long v = groupValue(fullGroupSet, entry.getKey());
+      for (Map.Entry<Pair<ImmutableBitSet, Integer>, Integer> entry : 
filters.entrySet()) {
+        final long v = groupValue(fullGroupSet, entry.getKey().left);
+        int distinctFilterArg = remap(fullGroupSet, entry.getKey().right);
+        RexNode expr = relBuilder.equals(nodeZ, relBuilder.literal(v));
+        if (distinctFilterArg > -1) {
+          // merge the filter of the distinct aggregate call itself.
+          RexBuilder rexBuilder = aggregate.getCluster().getRexBuilder();
+          expr = relBuilder.and(expr,
+              rexBuilder.makeCall(SqlStdOperatorTable.IS_TRUE,
+                  relBuilder.field(distinctFilterArg)));
+        }
         nodes.add(
-            relBuilder.alias(
-                relBuilder.equals(nodeZ, relBuilder.literal(v)),
-                "$g_" + v));
+            relBuilder.alias(expr,
+            "$g_" + v + (distinctFilterArg < 0 ? "" : "_" + 
distinctFilterArg)));
       }
 
 Review comment:
   I would suggest to use `$g_v_f_distinctFilterArg` for the alias, the `f` 
means filter.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to