[GitHub] [calcite] hsyuan commented on a change in pull request #1252: [CALCITE-3087] AggregateOnProjectToAggregateUnifyRule ignores Project incorrectly when its Mapping breaks ordering

[GitBox <gi...@apache.org>]

hsyuan commented on a change in pull request #1252: [CALCITE-3087] AggregateOnProjectToAggregateUnifyRule ignores Project incorrectly when its Mapping breaks ordering
URL: https://github.com/apache/calcite/pull/1252#discussion_r291756796
 
 

 ##########
 File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
 ##########
 @@ -1342,8 +1344,28 @@ public UnifyResult apply(UnifyRuleCall call) {
       }
       final MutableAggregate aggregate2 =
           permute(query, project.getInput(), mapping.inverse());
-      final MutableRel result = unifyAggregates(aggregate2, target);
-      return result == null ? null : call.result(result);
+      final MutableRel unifiedAggregate = unifyAggregates(aggregate2, target);
+      if (unifiedAggregate == null) {
+        return null;
+      }
+
+      MutableRel result = unifiedAggregate;
+
+      //add Project if the mapping breaks order of fields in GroupSet
 
 Review comment:
   `//add...` -> `// Add...`. Note add space after `//`, and capital letter.

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


With regards,
Apache Git Services