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 > > >
