[jira] [Commented] (FLINK-6257) Post-pass OVER windows

["sunjincheng (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLINK-6257?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15956221#comment-15956221 ] 

sunjincheng commented on FLINK-6257:
------------------------------------

Hi [~fhueske] I like this proposal very much. We really need to check the field of ORDER BY to ensure that the user is clearly aware of their behavior. And to ensure that the achieve logical of over window achieve is rigorous. 
IMO. In order to check  ORDER BY clause. We have two phases can be checked. First one is in {{planner.validate}} phase, In this phase will check the legitimacy of the `SqlWindow`，Then we can extend this validation to increase the validation of ORDER BY. But perhaps this is more difficult, because due to calcite visibility issues, sql window some properties FLINK cannot get to. Second one is in {{DataStreamOverAggregate/DataStreamOverAggregateRule}} phase, In this phase is very easy to do the checking. Because we can get the  TimeType
{{inputType.getFieldList.get(overWindow.orderKeys.getFieldCollations.get(0).getFieldIndex).getValue}} and orderType {{overWindow.orderKeys.getFieldCollations.get(0).direction}}. What do you think?

> Post-pass OVER windows
> ----------------------
>
>                 Key: FLINK-6257
>                 URL: https://issues.apache.org/jira/browse/FLINK-6257
>             Project: Flink
>          Issue Type: Sub-task
>          Components: Table API & SQL
>    Affects Versions: 1.3.0
>            Reporter: Fabian Hueske
>            Priority: Critical
>
> The OVER windows have been implemented by several contributors.
> We should do a post pass over the contributed code and:
> * Functionality
> ** currently every time attributes is allows as ORDER BY attribute. We must check that this is actually a time indicator ({{procTime()}}, {{rowTime()}}) an that the order is ASCENDING.
> * Documentation
> ** Add documentation for OVER windows
> * Code style
> ** Consistent naming of {{ProcessFunctions}} and methods
> * Tests
> ** Move the OVER window tests out of SqlITCase into a dedicated class
> ** Move the OVER window tests out of WindowAggregateTest into a dedicated class
> ** Add tests based on the test harness for all {{ProcessFunctions}} similar to {{BoundedProcessingOverRangeProcessFunction}}. The tests should include exact boundary checks for range windows and check for proper parallelization with multiple keys.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)