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
