Hi Jane, Thanks for the updates!
Just one small comment on the options in IncrementalAggregateRule & RelNodeBlock, should we also change the API level from Experimental to PublicEvolving? Best, Lincoln Lee Jane Chan <qingyue....@gmail.com> 于2024年5月21日周二 16:41写道: > Hi all, > > Thanks for your valuable feedback! > > To @Xuannan > > For options to be moved to another module/package, I think we have to > > mark the old option deprecated in 1.20 for it to be removed in 2.0, > > according to the API compatibility guarantees[1]. We can introduce the > > new option in 1.20 with the same option key in the intended class. > > > Good point, fixed. > > To @Lincoln and @Benchao > > Thanks for sharing the insights into the historical context of which I was > unaware. I've reorganized the sheet. > > 3. Regarding WindowEmitStrategy, IIUC it is currently unsupported on TVF > > window, so it's recommended to keep it untouched for now and follow up in > > FLINK-29692 > > > How to tackle the configuration is up to whether to remove the legacy > window aggregate in 2.0, and I've updated the FLIP to leverage this part to > FLINK-29692. > > Please let me know if that answers your questions or if you have other > comments. > > Best, > Jane > > > On Mon, May 20, 2024 at 1:52 PM Ron Liu <ron9....@gmail.com> wrote: > > > Hi, Lincoln > > > > > 2. Regarding the options in HashAggCodeGenerator, since this new > feature > > has gone > > through a couple of release cycles and could be considered for > > PublicEvolving now, > > cc @Ron Liu <ron9....@gmail.com> WDYT? > > > > Thanks for cc'ing me, +1 for public these options now. > > > > Best, > > Ron > > > > Benchao Li <libenc...@apache.org> 于2024年5月20日周一 13:08写道: > > > > > I agree with Lincoln about the experimental features. > > > > > > Some of these configurations do not even have proper implementation, > > > take 'table.exec.range-sort.enabled' as an example, there was a > > > discussion[1] about it before. > > > > > > [1] https://lists.apache.org/thread/q5h3obx36pf9po28r0jzmwnmvtyjmwdr > > > > > > Lincoln Lee <lincoln.8...@gmail.com> 于2024年5月20日周一 12:01写道: > > > > > > > > Hi Jane, > > > > > > > > Thanks for the proposal! > > > > > > > > +1 for the changes except for these annotated as experimental ones. > > > > > > > > For the options annotated as experimental, > > > > > > > > +1 for the moving of IncrementalAggregateRule & RelNodeBlock. > > > > > > > > For the rest of the options, there are some suggestions: > > > > > > > > 1. for the batch related parameters, it's recommended to either > delete > > > > them (leaving the necessary defaults value in place) or leave them as > > > they > > > > are. Including: > > > > FlinkRelMdRowCount > > > > FlinkRexUtil > > > > BatchPhysicalSortRule > > > > JoinDeriveNullFilterRule > > > > BatchPhysicalJoinRuleBase > > > > BatchPhysicalSortMergeJoinRule > > > > > > > > What I understand about the history of these options is that they > were > > > once > > > > used for fine > > > > tuning for tpc testing, and the current flink planner no longer > relies > > on > > > > these internal > > > > options when testing tpc[1]. In addition, these options are too > obscure > > > for > > > > SQL users, > > > > and some of them are actually magic numbers. > > > > > > > > 2. Regarding the options in HashAggCodeGenerator, since this new > > feature > > > > has gone > > > > through a couple of release cycles and could be considered for > > > > PublicEvolving now, > > > > cc @Ron Liu <ron9....@gmail.com> WDYT? > > > > > > > > 3. Regarding WindowEmitStrategy, IIUC it is currently unsupported on > > TVF > > > > window, so > > > > it's recommended to keep it untouched for now and follow up in > > > > FLINK-29692[2]. cc @Xuyang <xyzhong...@163.com> > > > > > > > > [1] > > > > > > > > > > https://github.com/ververica/flink-sql-benchmark/blob/master/tools/common/flink-conf.yaml > > > > [2] https://issues.apache.org/jira/browse/FLINK-29692 > > > > > > > > > > > > Best, > > > > Lincoln Lee > > > > > > > > > > > > Yubin Li <lyb5...@gmail.com> 于2024年5月17日周五 10:49写道: > > > > > > > > > Hi Jane, > > > > > > > > > > Thank Jane for driving this proposal ! > > > > > > > > > > This makes sense for users, +1 for that. > > > > > > > > > > Best, > > > > > Yubin > > > > > > > > > > On Thu, May 16, 2024 at 3:17 PM Jark Wu <imj...@gmail.com> wrote: > > > > > > > > > > > > Hi Jane, > > > > > > > > > > > > Thanks for the proposal. +1 from my side. > > > > > > > > > > > > > > > > > > Best, > > > > > > Jark > > > > > > > > > > > > On Thu, 16 May 2024 at 10:28, Xuannan Su <suxuanna...@gmail.com> > > > wrote: > > > > > > > > > > > > > Hi Jane, > > > > > > > > > > > > > > Thanks for driving this effort! And +1 for the proposed > changes. > > > > > > > > > > > > > > I have one comment on the migration plan. > > > > > > > > > > > > > > For options to be moved to another module/package, I think we > > have > > > to > > > > > > > mark the old option deprecated in 1.20 for it to be removed in > > 2.0, > > > > > > > according to the API compatibility guarantees[1]. We can > > introduce > > > the > > > > > > > new option in 1.20 with the same option key in the intended > > class. > > > > > > > WDYT? > > > > > > > > > > > > > > Best, > > > > > > > Xuannan > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > https://nightlies.apache.org/flink/flink-docs-master/docs/ops/upgrading/#api-compatibility-guarantees > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, May 15, 2024 at 6:20 PM Jane Chan < > qingyue....@gmail.com > > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > I'd like to start a discussion on FLIP-457: Improve Table/SQL > > > > > > > Configuration > > > > > > > > for Flink 2.0 [1]. This FLIP revisited all Table/SQL > > > configurations > > > > > to > > > > > > > > improve user-friendliness and maintainability as Flink moves > > > toward > > > > > 2.0. > > > > > > > > > > > > > > > > I am looking forward to your feedback. > > > > > > > > > > > > > > > > Best regards, > > > > > > > > Jane > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=307136992 > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Best, > > > Benchao Li > > > > > >