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

Reply via email to