Thanks yuxia for your explanation.

But what I mean is that this may lead to confusion for implementers
and users. You can use comments to explain it. However, a good
interface can make the mechanism clearer through code design.

So here, I still think an independent SupportsXX interface can make
the behavior more clear.

Best,
Jingsong

On Wed, Jan 4, 2023 at 10:56 AM yuxia <luoyu...@alumni.sjtu.edu.cn> wrote:
>
> Hi, Jingsong, thanks for your comments.
>
> ## About RowLevelDeleteMode
> That's really a good suggestion, now I have updated the FLIP to make 
> RowLevelDeleteMode a higer level.
>
> ## About scope of addContextParameter
> Sorry for the confusing, now I have updated the FLIP to add more comments for 
> it. The scope for the parameters is limited to the phase
> that Flink translates ranslates physical RelNode to ExecNode.
> It's possible to see all the other sources and sinks in a topo. For the 
> order, if only one sink, the sink will be last one to see the parametes,
> the order for the sources is consistent to the order the table source nodes 
> are translated to ExecNode.
> If there're multiple sinks for the case of StatementSet, the sink may also 
> see the parameters added by the table sources that belong the statment
> added earlier.
>
> ## About scope of getScanPurpose
> Yes, all sources wil see this method. But it won't bring any compatibility 
> issues for in here we just tell the source scan
> what's scan purpose without touching any other logic. If sources ignore this 
> method, it just works as normally. So I think there's
> no necessary to add a new interface like SupportsXX.
>
> Best regards,
> Yuxia
>
> ----- 原始邮件 -----
> 发件人: "Jingsong Li" <jingsongl...@gmail.com>
> 收件人: "dev" <dev@flink.apache.org>
> 发送时间: 星期二, 2023年 1 月 03日 下午 12:12:12
> 主题: Re: [DISCUSS] FLIP-282: Introduce Delete & Update API
>
> Thanks yuxia for the FLIP! It looks really good!
>
> I have three comments:
>
> ## RowLevelDeleteMode
>
> Can RowLevelDeleteMode be a higher level?
> `SupportsRowLevelDelete.RowLevelDeleteMode` is better than
> `SupportsRowLevelDelete.RowLevelDeleteInfo.RowLevelDeleteMode`.
> Same as `RowLevelUpdateMode`.
>
> ## Scope of addContextParameter
>
> I see that some of your comments are for sink, but can you make it
> clearer here? What exactly is its scope? For example, is it possible
> to see all the other sources and sinks in a topo? What is the order of
> seeing?
>
> ## Scope of getScanPurpose
>
> Will all sources see this method? Will there be compatibility issues?
> If sources ignore this method, will this cause strange phenomena?
>
> What I mean is: should another SupportsXX be created here to provide
> delete and update.
>
> Best,
> Jingsong
>
> On Thu, Dec 29, 2022 at 6:23 PM yuxia <luoyu...@alumni.sjtu.edu.cn> wrote:
> >
> > Hi, Lincoln Lee;
> > 1: Yes,  it's a typo; Thanks for pointing out. I have fixed the typo.
> > 2: For stream users,  assuming for delete, they will receive 
> > TableException("DELETE TABLE is not supported for streaming mode now"); 
> > Update is similar. I also update them to the FLIP.
> >
> > Best regards,
> > Yuxia
> >
> > ----- 原始邮件 -----
> > 发件人: "Lincoln Lee" <lincoln.8...@gmail.com>
> > 收件人: "dev" <dev@flink.apache.org>
> > 发送时间: 星期三, 2022年 12 月 28日 上午 9:50:50
> > 主题: Re: [DISCUSS] FLIP-282: Introduce Delete & Update API
> >
> > Hi yuxia,
> >
> > Thanks for the proposal! I think it'll be very useful for users in batch
> > scenarios to cooperate with external systems.
> >
> > For the flip I have two questions:
> > 1. Is it a typo the default method 'default ScanPurpose getScanPurpose();'
> > without implementation in interface ScanContext?
> > 2. For stream users, what exceptions will be received for this unsupported
> > operations?
> >
> > Best,
> > Lincoln Lee
> >
> >
> > yuxia <luoyu...@alumni.sjtu.edu.cn> 于2022年12月26日周一 20:24写道:
> >
> > > Hi, devs.
> > >
> > > I'd like to start a discussion about FLIP-282: Introduce Delete & Update
> > > API[1].
> > >
> > > Row-Level SQL Delete & Update are becoming more and more important in
> > > modern big data workflow. The use cases include deleting a set of rows for
> > > regulatory compliance, updating a set of rows for data correction, etc.
> > > So, in this FLIP, I want to introduce Delete & Update API to Flink in
> > > batch mode. With these interfaces, the external connectors will have
> > > ability to delete & update existing data in the corresponding storages.
> > >
> > > Looking forwards to your feedback.
> > >
> > > [1]:
> > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=235838061
> > >
> > >
> > > Best regards,
> > > Yuxia
> > >
> > >

Reply via email to