Fabian Hueske commented on CALCITE-2216:

Hi [~suez1224],

For the fix in Flink we can use the fact that the window properties are 
appended at the end of the output schema (grouping cols, aggregates, window 
Hence, all fields in the RelDataType that exceed the list of RexNodes are 
window properties and we can just create field access expressions for them.
We do not need to look up the RelNode from the builder.

I'm fine either way passing the LogicalAggregate or RelDataType.

Thanks, Fabian

> Improve extensibility of AggregateReduceFunctionsRule
> -----------------------------------------------------
>                 Key: CALCITE-2216
>                 URL: https://issues.apache.org/jira/browse/CALCITE-2216
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>    Affects Versions: 1.15.0
>            Reporter: Fabian Hueske
>            Assignee: Julian Hyde
>            Priority: Minor
> 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.

This message was sent by Atlassian JIRA

Reply via email to