[CALCITE-2216] Improve extensibility of AggregateReduceFunctionsRule

• Type: Improvement
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 1.15.0
• Fix Version/s: 1.17.0
• Component/s: core
• Assignee: Shuyi Chen
• Reporter: Fabian Hueske
• Created: 16/Mar/18 14:45
• Updated: 20/Jul/18 08:18
• Resolved: 09/Apr/18 18:22

Description

I'm proposing to improve the extensibility of AggregateReduceFunctionsRule. The purpose of the rule is to decompose complex aggregation functions like VAR_POP and STDDEV_SAMP into COUNT and SUM functions and compute the original functions in a subsequent Calc operator.

Right now, the rule class provides a protected method that can be overridden to create an Aggregate with the updated aggregate calls.

We are using the rule in Flink and have a special Aggregate Rel for group-windowed aggregations (GROUP BY TUMBLE/HOP/SESSION). Our implementation requires to forward some additional fields from the Aggregate for window properties like TUMBLE_START or HOP_END. In the current form, we cannot extend the rule, because these fields are striped off by the Calc node that is automatically added by the rule.

I'm proposing to also move the code to create the Calc into a protected method just like the code to create the new Aggregate.

I know, this is a fairly Flink-specific issue, but the code changes are minimal (no change in functionality) and it would help us, because we would not need to copy the rule and maintain it in Flink.

I'll open a PR for this. Looking forward to your comments.

Issue Links

• blocks: FLINK-9135 Remove AggregateReduceFunctionsRule once CALCITE-2216 is fixed