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