Hi, Jane.
Thanks for your feedback.
>1. Let's set aside the workload for a moment and clearly conceptualize the
>ideal implementation in the end state, or at the very least, document the
>current limitations in this FLIP to prevent them from being overlooked.
Actually the work that supports unaligned window in window agg operator is
included in this flip. We will always try to implement
features in window agg operator first and then consider falling back to legacy
group window agg if there are too many problems.
>2. Can we consider maintaining consistency at the ExecNode level for the
>session window TVF with other TVFs?
You're right, thanks for your reminder. I'll move the logic about fallback to
legacy legacy group window agg inside the exec node instead of physical node.
--
Best！
Xuyang
At 2023-12-11 20:38:25, "Jane Chan" <qingyue....@gmail.com> wrote:
>Hi Xuyang,
>
>Thanks for driving this discussion.
>
>I briefly reviewed the first PoC code[1] you provided for implementing the
>session window TVF, and I noticed a problem. In the `translateToExecNode`
>method of `StreamPhysicalWindowAggregate`, the session window TVF is
>translated into the legacy `StreamExecGroupWindowAggregate` instead of
>`StreamExecWindowAggregate`. I can guess that the reason for this is
>because we currently lack an operator that supports unaligned windows (and
>session windows belong to this category), but this means that we are only
>supporting session window TVF at the syntax level. My concern is that, if
>we support it in this way, it will be very difficult to get rid of the
>legacy implementation in forthcoming 2.0.
>
>Certainly, I understand that supporting it at the operator level would
>significantly increase the scope of work for this FLIP. Here my cents:
>
>1. Let's set aside the workload for a moment and clearly conceptualize the
>ideal implementation in the end state, or at the very least, document the
>current limitations in this FLIP to prevent them from being overlooked.
>
>2. Can we consider maintaining consistency at the ExecNode level for the
>session window TVF with other TVFs? By doing so, even if we introduce the
>actual unaligned window operator in a future 2.x release, we could still
>ensure that the session TVF from the version it is supported can be
>successfully deserialized through the `CompiledPlan`.
>
>WDYT?
>
>Best,
>Jane
>
>[1] https://github.com/xuyangzhong/flink/tree/FLINK-24024
>
>On Mon, Dec 11, 2023 at 11:41 AM Xuyang <xyzhong...@163.com> wrote:
>
>> Hi, Jim.
>> >As a clarification, since FLINK-24204 is finishing up work from
>> >FLIP-145[1], do we need to discuss anything before you work out the
>> details
>> >of FLINK-24024 as a PR?
>> Which issue do you mean? It seems that FLINK-24204[1] is the issue with
>> table api&sql type system.
>>
>>
>> > I've got a PR up [3] for moving at least one of the classes you are
>> touching.
>> Nice work! Since we are not going to delete the legacy group window agg
>> operator actually, the only compatibility issue
>> may be that when using flink sql, the legacy group window agg operator
>> will be rewritten into new operators. Will these tests be affected about
>> this rewritten?
>>
>>
>>
>>
>> [1] https://issues.apache.org/jira/browse/FLINK-24204
>>
>>
>>
>>
>>
>>
>> --
>>
>> Best！
>> Xuyang
>>
>>
>>
>>
>>
>> At 2023-12-09 06:25:30, "Jim Hughes" <jhug...@confluent.io.INVALID> wrote:
>> >Hi Xuyang,
>> >
>> >As a clarification, since FLINK-24204 is finishing up work from
>> >FLIP-145[1], do we need to discuss anything before you work out the
>> details
>> >of FLINK-24024 as a PR?
>> >
>> >Relatedly, as that goes up for a PR, as part of FLINK-33421 [2], Bonnie
>> and
>> >I are working through migrating some of the JsonPlan Tests and ITCases to
>> >RestoreTests. I've got a PR up [3] for moving at least one of the classes
>> >you are touching. Let me know if I can share any details about that work.
>> >
>> >Cheers,
>> >
>> >Jim
>> >
>> >1.
>> >
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-145%3A+Support+SQL+windowing+table-valued+function#FLIP145:SupportSQLwindowingtablevaluedfunction-SessionWindows
>> >
>> >2. https://issues.apache.org/jira/browse/FLINK-33421
>> >3. https://github.com/apache/flink/pull/23886
>> >https://issues.apache.org/jira/browse/FLINK-33676
>> >
>> >On Tue, Nov 28, 2023 at 7:31 AM Xuyang <xyzhong...@163.com> wrote:
>> >
>> >> Hi all.
>> >> I'd like to start a discussion of FLIP-392: Deprecate the Legacy Group
>> >> Window Aggregation.
>> >>
>> >>
>> >> Although the current Flink SQL Window Aggregation documentation[1]
>> >> indicates that the legacy Group Window Aggregation
>> >> syntax has been deprecated, the new Window TVF Aggregation syntax has
>> not
>> >> fully covered all of the features of the legacy one.
>> >>
>> >>
>> >> Compared to Group Window Aggergation, Window TVF Aggergation has several
>> >> advantages, such as two-stage optimization,
>> >> support for standard GROUPING SET syntax, and so on. However, it needs
>> to
>> >> supplement and enrich the following features.
>> >>
>> >>
>> >> 1. Support for SESSION Window TVF Aggregation
>> >> 2. Support for consuming CDC stream
>> >> 3. Support for HOP window size with non-integer step length
>> >> 4. Support for configurations such as early fire, late fire and allow
>> >> lateness
>> >> (which are internal experimental configurations in Group Window
>> >> Aggregation and not public to users yet.)
>> >> 5. Unification of the Window TVF Aggregation operator in runtime at the
>> >> implementation layer
>> >> (In the long term, the cost to maintain the operators about Window TVF
>> >> Aggregation and Group Window Aggregation is too expensive.)
>> >>
>> >>
>> >> This flip aims to continue the unfinished work in FLIP-145[2], which is
>> to
>> >> fully enable the capabilities of Window TVF Aggregation
>> >> and officially deprecate the legacy syntax Group Window Aggregation, to
>> >> prepare for the removal of the legacy one in Flink 2.0.
>> >>
>> >>
>> >> I have already done some preliminary POC to validate the feasibility of
>> >> the related work in this flip as follows.
>> >> 1. POC for SESSION Window TVF Aggregation [3]
>> >> 2. POC for CUMULATE in Group Window Aggregation operator [4]
>> >> 3. POC for consuming CDC stream in Window Aggregation operator [5]
>> >>
>> >>
>> >> Looking forward to your feedback and thoughts!
>> >>
>> >>
>> >>
>> >> [1]
>> >>
>> https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/window-agg/
>> >>
>> >> [2]
>> >>
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-145%3A+Support+SQL+windowing+table-valued+function#FLIP145:SupportSQLwindowingtablevaluedfunction-SessionWindows
>> >> [3] https://github.com/xuyangzhong/flink/tree/FLINK-24024
>> >> [4]
>> >>
>> https://github.com/xuyangzhong/flink/tree/poc_legacy_group_window_agg_cumulate
>> >> [5]
>> >>
>> https://github.com/xuyangzhong/flink/tree/poc_window_agg_consumes_cdc_stream
>> >>
>> >>
>> >>
>> >> --
>> >>
>> >> Best！
>> >> Xuyang
>>
