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