Hi, Timo. Sorry for this noise.
What do you think about splitting the flip like this?
--
Best！
Xuyang
At 2023-12-15 10:05:32, "Xuyang" <xyzhong...@163.com> wrote:
>Hi, Timo, thanks for your advice.
>
>
>I am considering splitting the existing flip into two while leaving the
>existing flip (or without).
>One of them points to the completion of the operator about window tvf to
>support CDC (there are several
>small work items, such as window agg, window rank, window join, etc. Due to
>time constraints,
>the 1.19 version takes priority to complete the window agg). The other points
>to the HOP window tvf
>supports a size that is a non-integer multiple of the step. Once these two
>flips are basically completed
>in 1.19, we can consider officially deprecating the old group window agg
>syntax in the release note.
>
>
>WDYT?
>
>
>
>
>--
>
> Best！
> Xuyang
>
>
>
>
>
>At 2023-12-14 17:51:01, "Timo Walther" <twal...@apache.org> wrote:
>>Hi Xuyang,
>>
>> > I'm not spliting this flip is that all of these subtasks like session
>>window tvf and cdc support do not change the public interface and the
>>public syntax
>>
>>Given the length of this mailing list discussion and number of involved
>>people I would strongly suggest to simplify the FLIP and give it a
>>better title to make quicker progress. In general, we all seem to be on
>>the same page in what we want. And both session TVF support and the
>>deprecation of the legacy group windows has been voted already and
>>discussed thouroughly. The FLIP can purely focus on the CDC topic.
>>
>>Cheers,
>>Timo
>>
>>
>>On 14.12.23 08:35, Xuyang wrote:
>>> Hi, Timo, Sergey and Lincoln Lee. Thanks for your feedback.
>>>
>>>
>>>> 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.
>>> The main goal and topic of this Flip is to align the abilities between the
>>> legacy group window agg syntax and the new window tvf syntax,
>>> and then we can say that the legacy window syntax will be deprecated. IMO,
>>> although there are many misalignments about these two
>>> syntaxes, such as session window tvf, cdc support and so on, they are all
>>> the subtasks we need to do in this flip. Another reason I'm not
>>> spliting this flip is that all of these subtasks like session window tvf
>>> and cdc support do not change the public interface and the public
>>> syntax, the implements of them will only be in modules table-planner and
>>> table-runtime.
>>>
>>>
>>>> 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.
>>>
>>>
>>> Agree with you. This flip will not and should not expose these experimental
>>> flink conf to users. I list them in this flip just aims to show the
>>> misalignments about these two window syntaxes.
>>>
>>>
>>> Look for your thought.
>>>
>>>
>>>
>>>
>>> --
>>>
>>> Best！
>>> Xuyang
>>>
>>>
>>>
>>>
>>>
>>> At 2023-12-13 15:40:16, "Lincoln Lee" <lincoln.8...@gmail.com> wrote:
>>>> Thanks Xuyang driving this work! It's great that everyone agrees with the
>>>> work itself in this flip[1]!
>>>>
>>>> Regarding whether to split the flip or adjust the scope of this flip, I'd
>>>> like to share some thoughts:
>>>>
>>>> 1. About the title of this flip, what I want to say is that flip-145[2] had
>>>> marked the legacy group window deprecated in the documentation but the
>>>> functionality of the new syntax is not aligned with the legacy one.
>>>> This is not a user-friendly deprecation, so the initiation of this flip, as
>>>> I understand it, is for the formal deprecation of the legacy window, which
>>>> requires us to complete the functionality alignment.
>>>>
>>>> 2. Agree with Timo that we can process the late-fire/early-fire features
>>>> separately. These experimental parameters have not been officially opened
>>>> to users.
>>>> Considering the workload, we can focus more on this version.
>>>>
>>>> 3. I have no objection to splitting this flip if everyone feels that the
>>>> work included is too much.
>>>> Regarding the support of session tvf, it seems that the main problem is
>>>> that this part of the description occupies a large part of the flip,
>>>> causing some misunderstandings.
>>>> This is indeed a predetermined task in FLIP-145, just adding more
>>>> explanation about semantics. In addition, I saw the discussion history in
>>>> FLINK-24024[3], thanks Sergey for being willing to help driving this work
>>>> together.
>>>>
>>>> [1]
>>>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-392%3A+Deprecate+the+Legacy+Group+Window+Aggregation
>>>> [2]
>>>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-145%3A+Support+SQL+windowing+table-valued+function
>>>> [3] https://issues.apache.org/jira/browse/FLINK-24024
>>>>
>>>> Best,
>>>> Lincoln Lee
>>>>
>>>>
>>>> Sergey Nuyanzin <snuyan...@gmail.com> 于2023年12月13日周三 08:02写道：
>>>>
>>>>> 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
>>>>>
