thanks for summarising Timo +1 for splitting it in different FLIPs and agree about having "SESSION Window TVF Aggregation" under FLIP-145 Moreover the task is already there, so no need to move it from one FLIP to another
>And actually Sergey Nuyanzin wanted to work >in this if I remember correctly. Not sure if this is still the case. Yes, however it seems there is already an existing PR for that. Anyway I'm happy to help here with the review as well and will reserve some time for it in coming days. On Tue, Dec 12, 2023 at 12:18 PM Timo Walther <twal...@apache.org> wrote: > Hi Xuyang, > > thanks for proposing this FLIP. In my opinion the FLIP touches too many > topics at the same time and should be split into multiple FLIPs. We > should stay focused on what is needed for Flink 2.0. > > Let me summarizing the topics: > > 1) SESSION Window TVF Aggregation > > This has been agreed in FLIP-145 already. We don't need another FLIP but > someone that finally implements this after we have performed the Calcite > upgrade a couple of months ago. The Calcite upgrade was important > exactly for SESSION windows. And actually Sergey Nuyanzin wanted to work > in this if I remember correctly. Not sure if this is still the case. > > 2) CDC support of Window TVFs > > This can be a FLIP on its own. > > 3) HOP window size with non-integer step length > > This can be a FLIP on its own. > > 4) Configurations such as early fire, late fire and allow lateness > > Can we postpone this discussion? Currently we should focus on user > switching to Window TVFs before Flink 2.0. Early fire, late fire and > allow lateness have not exposed through public configuration. It can be > introduced after Flink 2.0 released. > > Regards, > Timo > > > On 12.12.23 08:01, Xuyang wrote: > > Hi, Jim. > > Thanks for your explaination. > >> Ah, I mean to ask if you can contribute the new SESSION Table support > >> without needing FLIP-392 completely settled. I was trying to see if > that > >> is separate work which can be done or if there is some dependency on > this > >> FLIP. > > The pr available about session window tvf belongs to this flip I think, > and is part of the work about this flip. Actually the poc pr is not ready > completely yet, > > I'll try to update it to implement the session window tvf in window agg > operator instead of using legacy group window agg operator. > >> The tests should not be impacted. Depending on what order our work > lands > >> in, one of the tests you've added/updated would likely move to the > >> RestoreTests that Bonnie and I are working on. Just mentioning that > ahead > >> of time > > > > Got it! I will pay attention to it. > > > > > > > > > > -- > > > > Best！ > > Xuyang > > > > > > > > > > > > 在 2023-12-11 21:35:00，"Jim Hughes" <jhug...@confluent.io.INVALID> 写道： > >> Hi Xuyang, > >> > >> On Sun, Dec 10, 2023 at 10:41 PM 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. > >>> > >> > >> Ah, I mean to ask if you can contribute the new SESSION Table support > >> without needing FLIP-392 completely settled. I was trying to see if > that > >> is separate work which can be done or if there is some dependency on > this > >> FLIP. > >> > >> > >>> 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? > >>> > >> > >> The tests should not be impacted. Depending on what order our work > lands > >> in, one of the tests you've added/updated would likely move to the > >> RestoreTests that Bonnie and I are working on. Just mentioning that > ahead > >> of time > >> > >> Cheers, > >> > >> Jim > >> > >> > >> > >>> > >>> [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 > >>> > > -- Best regards, Sergey
