[GitHub] [calcite] hsyuan commented on a change in pull request #1033: [CALCITE-2820] Add a version of AggregateReduceFunctionsRule that does not reduce SUM to SUM0

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

hsyuan commented on a change in pull request #1033: [CALCITE-2820] Add a version of AggregateReduceFunctionsRule that does not reduce SUM to SUM0
URL: https://github.com/apache/calcite/pull/1033#discussion_r263230791
 
 

 ##########
 File path: core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java
 ##########
 @@ -97,16 +100,66 @@
       new AggregateReduceFunctionsRule(operand(LogicalAggregate.class, any()),
           RelFactories.LOGICAL_BUILDER);
 
+  private final Set<SqlKind> functionsToReduce;
+
+  /**
+   * Gets an instance of AggregateReduceFunctionsRule
+   * with client provided specific functions to reduce
+   * @param functionsToReduce client provided information
+   *                          on which specific functions will be
+   *                          reduced by this rule
+   * @return an instance of AggregateReduceFunctionsRule
+   */
+  public static AggregateReduceFunctionsRule
+      getInstanceWithSpecificFunctionsToReduce(final Set<SqlKind> functionsToReduce) {
+    Preconditions.checkArgument(functionsToReduce != null,
+        "Error: expecting a valid handle for AggregateFunctionsToReduce");
+    return new AggregateReduceFunctionsRule(operand(LogicalAggregate.class, any()),
+      RelFactories.LOGICAL_BUILDER, functionsToReduce);
+  }
+
   //~ Constructors -----------------------------------------------------------
 
   /** Creates an AggregateReduceFunctionsRule. */
   public AggregateReduceFunctionsRule(RelOptRuleOperand operand,
       RelBuilderFactory relBuilderFactory) {
     super(operand, relBuilderFactory, null);
+    // by default, this rule will reduce all functions it handles
+    this.functionsToReduce = new HashSet<>();
+    addDefaultSetOfFunctionsToReduce();
+  }
+
+  /**
+   * Creates an AggregateReduceFunctionsRule with client
+   * provided information on which specific functions will
+   * be reduced by this rule
+   * @param operand root operand
+   * @param relBuilderFactory builder for relational expressions
+   * @param functionsToReduce client provided information
+   *                          on which specific functions
+   *                          will be reduced by this rule
+   */
+  public AggregateReduceFunctionsRule(RelOptRuleOperand operand,
+      RelBuilderFactory relBuilderFactory, Set<SqlKind> functionsToReduce) {
+    super(operand, relBuilderFactory, null);
+    this.functionsToReduce = functionsToReduce;
   }
 
   //~ Methods ----------------------------------------------------------------
 
+  private void addDefaultSetOfFunctionsToReduce() {
+    functionsToReduce.add(SqlKind.AVG);
+    functionsToReduce.add(SqlKind.SUM);
+    functionsToReduce.add(SqlKind.STDDEV_POP);
+    functionsToReduce.add(SqlKind.STDDEV_SAMP);
+    functionsToReduce.add(SqlKind.VAR_POP);
+    functionsToReduce.add(SqlKind.VAR_SAMP);
+    functionsToReduce.add(SqlKind.COVAR_POP);
+    functionsToReduce.add(SqlKind.COVAR_SAMP);
+    functionsToReduce.add(SqlKind.REGR_SXX);
+    functionsToReduce.add(SqlKind.REGR_SYY);
 
 Review comment:
   I think it may be better to put this into a static block, and make functionsToReduce a static member.

----------------------------------------------------------------
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