If we all agree the option should only be handled by sql client, then why
don't we
just call it `sql-client.dml-sync`? As you said, calling it
`table.dml-sync` but has no
affection in `TableEnv.executeSql("INSERT INTO")` will also cause a big
confusion for
users.
The only concern I saw is if we introduce
"TableEnvironment.executeMultiSql()" in the
future, how do we control the synchronization between statements? TBH I
don't really
see a strong requirement for such interfaces. Right now, we have a pretty
clear semantic
of `TableEnv.executeSql`, and it's very convenient for users if they want
to execute multiple
sql statements. They can simulate either synced or async execution with
this building block.
This will introduce slight overhead for users, but compared to the
confusion we might
cause if we introduce such a method of our own, I think it's better to wait
for some more
feedback.
Best,
Kurt
On Tue, Feb 23, 2021 at 9:45 PM Timo Walther <twal...@apache.org> wrote:
> Hi Kurt,
>
> we can also shorten it to `table.dml-sync` if that would help. Then it
> would confuse users that do a regular `.executeSql("INSERT INTO")` in a
> notebook session.
>
> In any case users will need to learn the semantics of this option.
> `table.multi-dml-sync` should be described as "If a you are in a multi
> statement environment, execute DMLs synchrounous.". I don't have a
> strong opinion on shortening it to `table.dml-sync`.
>
> Just to clarify the implementation: The option should be handled by the
> SQL Client only, but the name can be shared accross platforms.
>
> Regards,
> Timo
>
>
> On 23.02.21 09:54, Kurt Young wrote:
> > Sorry for the late reply, but I'm confused by `table.multi-dml-sync`.
> >
> > IIUC this config will take effect with 2 use cases:
> > 1. SQL client, either interactive mode or executing multiple statements
> via
> > -f. In most cases,
> > there will be only one INSERT INTO statement but we are controlling the
> > sync/async behavior
> > with "*multi-dml*-sync". I think this will confuse a lot of users.
> Besides,
> >
> > 2. TableEnvironment#executeMultiSql(), but this is future work, we are
> also
> > not sure if we will
> > really introduce this in the future.
> >
> > I would prefer to introduce this option for only sql client. For
> platforms
> > Timo mentioned which
> > need to control such behavior, I think it's easy and flexible to
> introduce
> > one on their own.
> >
> > Best,
> > Kurt
> >
> >
> > On Sat, Feb 20, 2021 at 10:23 AM Shengkai Fang <fskm...@gmail.com>
> wrote:
> >
> >> Hi everyone.
> >>
> >> Sorry for the late response.
> >>
> >> For `execution.runtime-mode`, I think it's much better than
> >> `table.execution.mode`. Thanks for Timo's suggestions!
> >>
> >> For `SHOW CREATE TABLE`, I'm +1 with Jark's comments. We should clarify
> the
> >> usage of the SHOW CREATE TABLE statements. It should be allowed to
> specify
> >> the table that is fully qualified and only works for the table that is
> >> created by the sql statements.
> >>
> >> I have updated the FLIP with suggestions. It seems we have reached a
> >> consensus, I'd like to start a formal vote for the FLIP.
> >>
> >> Please vote +1 to approve the FLIP, or -1 with a comment.
> >>
> >> Best,
> >> Shengkai
> >>
> >> Jark Wu <imj...@gmail.com> 于2021年2月15日周一 下午10:50写道：
> >>
> >>> Hi Ingo,
> >>>
> >>> 1) I think you are right, the table path should be fully-qualified.
> >>>
> >>> 2) I think this is also a good point. The SHOW CREATE TABLE
> >>> only aims to print DDL for the tables registered using SQL CREATE TABLE
> >>> DDL.
> >>> If a table is registered using Table API, e.g.
> >>> `StreamTableEnvironment#createTemporaryView(String, DataStream)`,
> >>> currently it's not possible to print DDL for such tables.
> >>> I think we should point it out in the FLIP.
> >>>
> >>> Best,
> >>> Jark
> >>>
> >>>
> >>>
> >>> On Mon, 15 Feb 2021 at 21:33, Ingo Bürk <i...@ververica.com> wrote:
> >>>
> >>>> Hi all,
> >>>>
> >>>> I have a couple questions about the SHOW CREATE TABLE statement.
> >>>>
> >>>> 1) Contrary to the example in the FLIP I think the returned DDL should
> >>>> always have the table identifier fully-qualified. Otherwise the DDL
> >>> depends
> >>>> on the current context (catalog/database), which could be surprising,
> >>>> especially since "the same" table can behave differently if created in
> >>>> different catalogs.
> >>>> 2) How should this handle tables which cannot be fully characterized
> by
> >>>> properties only? I don't know if there's an example for this yet, but
> >>>> hypothetically this is not currently a requirement, right? This isn't
> >> as
> >>>> much of a problem if this syntax is SQL-client-specific, but if it's
> >>>> general Flink SQL syntax we should consider this (one way or another).
> >>>>
> >>>>
> >>>> Regards
> >>>> Ingo
> >>>>
> >>>> On Fri, Feb 12, 2021 at 3:53 PM Timo Walther <twal...@apache.org>
> >> wrote:
> >>>>
> >>>>> Hi Shengkai,
> >>>>>
> >>>>> thanks for updating the FLIP.
> >>>>>
> >>>>> I have one last comment for the option `table.execution.mode`. Should
> >>> we
> >>>>> already use the global Flink option `execution.runtime-mode` instead?
> >>>>>
> >>>>> We are using Flink's options where possible (e.g. `pipeline.name`
> >> and
> >>>>> `parallism.default`) why not also for batch/streaming mode?
> >>>>>
> >>>>> The description of the option matches to the Blink planner behavior:
> >>>>>
> >>>>> ```
> >>>>> Among other things, this controls task scheduling, network shuffle
> >>>>> behavior, and time semantics.
> >>>>> ```
> >>>>>
> >>>>> Regards,
> >>>>> Timo
> >>>>>
> >>>>> On 10.02.21 06:30, Shengkai Fang wrote:
> >>>>>> Hi, guys.
> >>>>>>
> >>>>>> I have updated the FLIP. It seems we have reached agreement. Maybe
> >>> we
> >>>>> can
> >>>>>> start the vote soon. If anyone has other questions, please leave
> >> your
> >>>>>> comments.
> >>>>>>
> >>>>>> Best，
> >>>>>> Shengkai
> >>>>>>
> >>>>>> Rui Li <lirui.fu...@gmail.com>于2021年2月9日 周二下午7:52写道：
> >>>>>>
> >>>>>>> Hi guys,
> >>>>>>>
> >>>>>>> The conclusion sounds good to me.
> >>>>>>>
> >>>>>>> On Tue, Feb 9, 2021 at 5:39 PM Shengkai Fang <fskm...@gmail.com>
> >>>> wrote:
> >>>>>>>
> >>>>>>>> Hi, Timo, Jark.
> >>>>>>>>
> >>>>>>>> I am fine with the new option name.
> >>>>>>>>
> >>>>>>>> Best,
> >>>>>>>> Shengkai
> >>>>>>>>
> >>>>>>>> Timo Walther <twal...@apache.org>于2021年2月9日 周二下午5:35写道：
> >>>>>>>>
> >>>>>>>>> Yes, `TableEnvironment#executeMultiSql()` can be future work.
> >>>>>>>>>
> >>>>>>>>> @Rui, Shengkai: Are you also fine with this conclusion?
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>> Timo
> >>>>>>>>>
> >>>>>>>>> On 09.02.21 10:14, Jark Wu wrote:
> >>>>>>>>>> I'm fine with `table.multi-dml-sync`.
> >>>>>>>>>>
> >>>>>>>>>> My previous concern about "multi" is that DML in CLI looks like
> >>>>>>> single
> >>>>>>>>>> statement.
> >>>>>>>>>> But we can treat CLI as a multi-line accepting statements from
> >>>>>>> opening
> >>>>>>>> to
> >>>>>>>>>> closing.
> >>>>>>>>>> Thus, I'm fine with `table.multi-dml-sync`.
> >>>>>>>>>>
> >>>>>>>>>> So the conclusion is `table.multi-dml-sync` (false by default),
> >>> and
> >>>>>>> we
> >>>>>>>>> will
> >>>>>>>>>> support this config
> >>>>>>>>>> in SQL CLI first, will support it in
> >>>>>>> TableEnvironment#executeMultiSql()
> >>>>>>>>> in
> >>>>>>>>>> the future, right?
> >>>>>>>>>>
> >>>>>>>>>> Best,
> >>>>>>>>>> Jark
> >>>>>>>>>>
> >>>>>>>>>> On Tue, 9 Feb 2021 at 16:37, Timo Walther <twal...@apache.org>
> >>>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>>> Hi everyone,
> >>>>>>>>>>>
> >>>>>>>>>>> I understand Rui's concerns. `table.dml-sync` should not apply
> >>> to
> >>>>>>>>>>> regular `executeSql`. Actually, this option makes only sense
> >>> when
> >>>>>>>>>>> executing multi statements. Once we have a
> >>>>>>>>>>> `TableEnvironment.executeMultiSql()` this config could be
> >>>>>>> considered.
> >>>>>>>>>>>
> >>>>>>>>>>> Maybe we can find a better generic name? Other platforms will
> >>> also
> >>>>>>>> need
> >>>>>>>>>>> to have this config option, which is why I would like to
> >> avoid a
> >>>> SQL
> >>>>>>>>>>> Client specific option. Otherwise every platform has to come
> >> up
> >>>> with
> >>>>>>>>>>> this important config option separately.
> >>>>>>>>>>>
> >>>>>>>>>>> Maybe `table.multi-dml-sync` `table.multi-stmt-sync`? Or other
> >>>>>>>> opinions?
> >>>>>>>>>>>
> >>>>>>>>>>> Regards,
> >>>>>>>>>>> Timo
> >>>>>>>>>>>
> >>>>>>>>>>> On 09.02.21 08:50, Shengkai Fang wrote:
> >>>>>>>>>>>> Hi, all.
> >>>>>>>>>>>>
> >>>>>>>>>>>> I think it may cause user confused. The main problem is we
> >>> have
> >>>> no
> >>>>>>>>> means
> >>>>>>>>>>>> to detect the conflict configuration, e.g. users set the
> >> option
> >>>>>>> true
> >>>>>>>>> and
> >>>>>>>>>>>> use `TableResult#await` together.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Best,
> >>>>>>>>>>>> Shengkai.
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> Best regards!
> >>>>>>> Rui Li
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
>
>
