Hi Lincoln,
This FLIP is already in a good shape. +1. Thanks for driving it. I just
have a very tiny comment, e.g. NIT. The "wide table" you mentioned in
the FLIP should be "denormalized table"[1] and fact tables in data
warehouse are commonly denormalized tables[2]. As far as I am concerned,
it will be a little bit easier for users to understand the motivation
and where they can benefit from it, if well known terms are used,
especially if they might be described in Java doc or Flink doc.
Best regards,
Jing
[1]
https://medium.com/analytics-vidhya/database-normalization-vs-denormalization-a42d211dd891
[2] https://www.quora.com/Is-fact-table-a-normalized-or-denormalized-table
On 3/7/23 14:40, Lincoln Lee wrote:
Hi all,
Both the FLIP[1] and the poc[2] has been updated, using Optional<int[][]>
to replace the return value of `getTargetColumns`
[1]
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240885081
[2] https://github.com/apache/flink/pull/22041
Best,
Lincoln Lee
Jane Chan <qingyue....@gmail.com> 于2023年3月7日周二 17:56写道：
Hi Lincoln,
Thanks for bringing this to the discussion. +1 for the FLIP.
Best,
Jane
On Tue, Mar 7, 2023 at 3:23 PM Aitozi <gjying1...@gmail.com> wrote:
we can initiate corresponding support issues for
specific connectors to follow up on support after finalizing the API
changes
Make sense to me!
Best,
Aitozi.
Lincoln Lee <lincoln.8...@gmail.com> 于2023年3月7日周二 15:05写道：
Thanks Jingsong & Hang,
Using Optional<int[][]> as the return value is a good idea.
Previously, I
hoped to keep the style of the LookupTableSource.LookupContext#getKeys
as
consistent as possible, but the getKeys is actually non-empty when
used,
so
I support updating to Optional<int[][]>. I'll update the flip doc and
poc
later tonight.
Best,
Lincoln Lee
Lincoln Lee <lincoln.8...@gmail.com> 于2023年3月7日周二 14:53写道：
Hi Aitozi,
Thanks for your feedback! Yes, including HBase and JDBC connector,
they
can be considered for support in the next step (JDBC as as a standard
protocol supported not only in traditional databases, but also in
more
and
more new types of storage). Considering the ongoing externalizing of
connectors and the release cycles of the connectors are decoupled
with
the
release cycle of Flink, we can initiate corresponding support issues
for
specific connectors to follow up on support after finalizing the API
changes, WDYT?
Best,
Lincoln Lee
Hang Ruan <ruanhang1...@gmail.com> 于2023年3月7日周二 12:14写道：
Hi, Lincoln,
Thanks for bringing this up. It looks good to me. I also agree with
Jingsong's suggestion.
Best,
Hang
Jingsong Li <jingsongl...@gmail.com> 于2023年3月7日周二 11:15写道：
Wow, we have 300 FLIPs...
Thanks Lincoln,
Have you considered returning an Optional<int[][]>?
Empty array looks a little weird to me.
Best,
Jingsong
On Tue, Mar 7, 2023 at 10:32 AM Aitozi <gjying1...@gmail.com>
wrote:
Hi Lincoln,
Thank you for sharing this FLIP. Overall, it looks good to
me. I
have
one question: with the introduction of this interface,
will any existing Flink connectors need to be updated in order
to
take
advantage of its capabilities? For example, HBase.
yuxia <luoyu...@alumni.sjtu.edu.cn> 于2023年3月7日周二 10:01写道：
Thanks. It makes sense to me.
Best regards,
Yuxia
----- 原始邮件 -----
发件人: "Lincoln Lee" <lincoln.8...@gmail.com>
收件人: "dev" <dev@flink.apache.org>
发送时间: 星期一, 2023年 3 月 06日 下午 10:26:26
主题: Re: [DISCUSS] FLIP-300: Add targetColumns to
DynamicTableSink#Context
to solve the null overwrite problem of partial-insert
hi yuxia,
Thanks for your feedback and tracking the issue of update
statement!
I've
updated the FLIP[1] and also the poc[2].
Since the bug and flip are orthogonal, we can focus on
finalizing
the
api
changes first, and then work on the flip implementation and
bugfix
separately, WDYT?
[1]
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240885081
[2] https://github.com/apache/flink/pull/22041
Best,
Lincoln Lee
yuxia <luoyu...@alumni.sjtu.edu.cn> 于2023年3月6日周一 21:21写道：
Hi, Lincoln.
Thanks for bringing this up. +1 for this FLIP, it's helpful
for
external
storage system to implement partial update.
The FLIP looks good to me. I only want to add one comment,
update
statement also doesn't support updating nested column, I
have
created
FLINK-31344[1] to track it.
Maybe we also need to explain it in this FLIP.
[1] https://issues.apache.org/jira/browse/FLINK-31344
Best regards,
Yuxia
----- 原始邮件 -----
发件人: "Lincoln Lee" <lincoln.8...@gmail.com>
收件人: "dev" <dev@flink.apache.org>
发送时间: 星期五, 2023年 3 月 03日 下午 12:22:19
主题: [DISCUSS] FLIP-300: Add targetColumns to
DynamicTableSink#Context to
solve the null overwrite problem of partial-insert
Hi everyone,
This FLIP[1] aims to support connectors in avoiding
overwriting
non-target
columns with null values when processing partial column
updates,
we
propose
adding information on the target column list to
DynamicTableSink#Context.
FLINK-18726[2] supports inserting statements with specified
column
list,
it
fills null values (or potentially declared default values in
the
future)
for columns not appearing in the column list of insert
statement
to
the
target table.
But this behavior does not satisfy some partial column
update
requirements
of some storage systems which allow storing null values. The
problem
is
that connectors cannot distinguish whether the null value
of a
column is
really from the user's data or whether it is a null value
populated
because
of partial insert behavior.
Looking forward to your comments or feedback.
[1]
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240885081
[2] https://issues.apache.org/jira/browse/FLINK-18726
Best,
Lincoln Lee
