siddharthteotia 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_r263125111
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java
##########
@@ -97,12 +98,96 @@
new AggregateReduceFunctionsRule(operand(LogicalAggregate.class, any()),
RelFactories.LOGICAL_BUILDER);
+ private final AggregateFunctionsToReduce functionsToReduce;
+
+ /**
+ * Used to create a non-default version of this rule
+ * where we can be specific about which functions
+ * should be reduced by the rule
+ */
+ public static class AggregateFunctionsToReduce {
+ private final boolean reduceAvg;
+ private final boolean reduceSum;
+ private final boolean reduceStddevPop;
+ private final boolean reduceStddevSamp;
+ private final boolean reduceVarPop;
+ private final boolean reduceVarSamp;
+ private final boolean reduceCovarPop;
+ private final boolean reduceCovarSamp;
+ private final boolean reduceRegrSxx;
+ private final boolean reduceRegrSyy;
+
+ private AggregateFunctionsToReduce() {
+ this.reduceAvg = true;
+ this.reduceSum = true;
+ this.reduceStddevPop = true;
+ this.reduceStddevSamp = true;
+ this.reduceVarPop = true;
+ this.reduceVarSamp = true;
+ this.reduceCovarPop = true;
+ this.reduceCovarSamp = true;
+ this.reduceRegrSxx = true;
+ this.reduceRegrSyy = true;
+ }
+
+ public AggregateFunctionsToReduce(final boolean reduceAvg, final boolean
reduceSum,
+ final boolean reduceStddevPop, final boolean reduceStddevSamp,
+ final boolean reduceVarPop, final boolean reduceVarSamp,
+ final boolean reduceCovarPop, final boolean reduceCovarSamp,
+ final boolean reduceRegrSxx, final boolean reduceRegrSyy) {
+ this.reduceAvg = reduceAvg;
+ this.reduceSum = reduceSum;
+ this.reduceStddevPop = reduceStddevPop;
+ this.reduceStddevSamp = reduceStddevSamp;
+ this.reduceVarPop = reduceVarPop;
+ this.reduceVarSamp = reduceVarSamp;
+ this.reduceCovarPop = reduceCovarPop;
+ this.reduceCovarSamp = reduceCovarSamp;
+ this.reduceRegrSxx = reduceRegrSxx;
+ this.reduceRegrSyy = reduceRegrSyy;
+ }
+ }
+
+ /**
+ * 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
AggregateFunctionsToReduce functionsToReduce) {
Review comment:
The reason why I used a static factory method is because I wasn't sure if
the callers will always be able to create a constructor themselves if they
don't have knowledge of the arguments RelOptRuleOperand and RelBuilderFactory.
Until now the callers always grab instance as
AggregateReduceFunctionsRule.INSTANCE and they will continue to do so to get
the default instance of this rule that reduces all functions.
For callers interested in reducing specific functions, they can call the
factory method with Set<SqlKind> functionsToReduce.
I am just not sure if the knowledge of the constructor's arguments is there
in the client. Like do they always know what to pass in for RelOptRuleOperand
and RelBuilderFactory. So I thought public factory method is the way to go. Is
that fine?
----------------------------------------------------------------
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
