>From my point of view, I also prefer "sql-client.dml-sync", because the behavior of this configuration is very clear. Even if we introduce a new config in the future, e.g. `table.dml-sync`, we can also deprecate the sql-client one.
Introducing a "table." configuration without any implementation will confuse users a lot, as they expect it should take effect on the Table API. If we want to introduce an unified "table.dml-sync" option, I prefer it should be implemented on Table API and affect all the DMLs on Table API (`tEnv.executeSql`, `Table.executeInsert`, `StatementSet`), as I have mentioned before [1]. > It would be very straightforward that it affects all the DMLs on SQL CLI and TableEnvironment (including `executeSql`, `StatementSet`, `Table#executeInsert`, etc.). This can also make SQL CLI easy to support this configuration by passing through to the TableEnv. Best, Jark [1]: http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-FLIP-163-SQL-Client-Improvements-tp48354p48665.html On Wed, 24 Feb 2021 at 10:39, Kurt Young <ykt...@gmail.com> wrote: > 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 > > >>>>>>> > > >>>>>> > > >>>>> > > >>>>> > > >>>> > > >>> > > >> > > > > > > > >
