Thanks Jane for the refine work, +1 from my side. I adjusted the table format of FLIP so that it can display all content in one page.
Best, Leonard > 2024年5月22日 下午3:42，Jane Chan <qingyue....@gmail.com> 写道： > > Hi Lincoln, > > Thanks for your suggestion. I've reviewed the comments from the previous PR > review[1], and the agreement at the time was that any configuration options > not included in ExecutionConfigOptions and OptimizerConfigOptions should > have the Experimental annotation explicitly added. Since this annotation > has been relatively stable from 1.9.0 until now, you make a valid point, > and we can elevate it to the PublicEvolving level. > > Please let me know if you have any questions. > > [1] https://github.com/apache/flink/pull/8980 > > Best, > Jane > > On Tue, May 21, 2024 at 10:25 PM Lincoln Lee <lincoln.8...@gmail.com> wrote: > >> 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 >>>>> >>>> >>> >>
