Hi Timo, Thanks for your feedback.
The reason of `The DDL can like this (With hive dialect)` is: The syntax of creating partition table is controversial, so we think we should put it aside for the time being to make it invisible to users. Since we implemented this syntax in 1.9, we decided to put it under hive dialect. (Subsequent votes will be taken to determine.) MSCK means metastore check, It is listed in the document mainly for the integrity of the story. Of course, it can be ignored. I fully agree that it can be discussed slowly in the future. >The current problem for the CLI is that it still does not use the flink-sql-parser. We can support all new syntax in Flink 1.11 once the CLI architecture has changed there. With the current architecture (using simple regex), I'm not sure if we can support all the grammar listed in the FLIP. You are right, we can not support any DDL grammars in FLIP. we should wait refactor of SQL-CLI in 1.11. What we can do is to remove the restriction of hiveDialect for the insert (overwrite and partition) syntax. That's what we've supported, and so far we haven't received any objections. Best, Jingsong Lee On Fri, Dec 13, 2019 at 4:51 PM Timo Walther <twal...@apache.org> wrote: > Hi everyone, > > sorry, I was not aware that FLIP-63 already lists a lot of additional > SQL grammar. It was accepted though an official voting process so I > guess we can adopt the listed grammar for Flink SQL. > > The only thing that confuses me is the mentioning of `The DDL can like > this (With hive dialect)`, for the next time we should make it more > explicit what belongs to the Flink SQL dialect and what belongs to the > Hive dialect. Also the not intuitive `MSCK REPAIR TABLE` seems strange > to me. What does MSCK stand for? Maybe we should skip stuff like that > for now. > > The current problem for the CLI is that it still does not use the > flink-sql-parser. We can support all new syntax in Flink 1.11 once the > CLI architecture has changed there. With the current architecture (using > simple regex), I'm not sure if we can support all the grammar listed in > the FLIP. > > Regards, > Timo > > On 13.12.19 04:22, Rui Li wrote: > > Hi Timo, > > > > I understand we need further discussion about syntax/dialect for 1.11. > But > > as Jark has pointed out, the current implementation violates the accepted > > design of FLIP-63, which IMO qualifies as a bug. Given that it's a bug > and > > has great impact on the usability of our Hive integration, do you think > we > > can fix it in 1.10? > > > > On Fri, Dec 13, 2019 at 12:24 AM Jingsong Li <jingsongl...@gmail.com> > wrote: > > > >> Hi Timo, > >> > >> I am OK if you think they are not bug and they should not be included in > >> 1.10. > >> > >> I think they have been accepted in FLIP-63. And there is no objection. > It > >> has been more than three months since the discussion of FLIP-63. It's > been > >> six months since Flink added these two syntaxs. > >> > >> But I can also start discussion and vote thread for FLIP-63 again, to > make > >> sure once again that everyone is happy. > >> > >> Best, > >> Jingsong Lee > >> > >> On Thu, Dec 12, 2019 at 11:35 PM Timo Walther <twal...@apache.org> > wrote: > >> > >>> Hi Jingsong, > >>> > >>> I will also add my opinion here for future discussions: > >>> > >>> We had long discussions around SQL syntax in the past (e.g. for > >>> WATERMARK or the concept of SYSTEM/TEMPORARY catalog objects) but in > the > >>> end all parties were happy and we came up with a good long-term > solution > >>> that is unlikely to be changed in the near future. IMHO we can differ > >>> from the standard esp. for DDL statements (every SQL vendors has custom > >>> syntax there) but still we should have time to hear many opinions. > >>> However, for DQL and DML statements we need to be even more cautious > >>> because here we enter SQL standard areas. > >>> > >>> Happy to discuss a partion syntax for 1.11 and go with the solution > that > >>> Jark proposed using a config option. > >>> > >>> Thanks, > >>> Timo > >>> > >>> > >>> On 12.12.19 09:40, Jark Wu wrote: > >>>> Hi Jingsong, > >>>> > >>>> Thanks for the explanation, I think I misunderstood your point at the > >>>> begining. > >>>> As FLIP-63 proposed, INSERT OVERWRITE and INSERT PARTITION syntax are > >>> added > >>>> to Flink's > >>>> SQL syntax, but CREATE PARTITION TABLE should be limited under Hive > >>>> dialect. > >>>> However, the current implementation is opposite, INSERT OVERWRITE > >>>> and INSERT PARTITION are > >>>> under the dialect limitation, but CREATE PARTITION TABLE is not. > >>>> > >>>> So it is indeed a bug which should be fixed in 1.10. > >>>> > >>>> Best, > >>>> Jark > >>>> > >>>> On Thu, 12 Dec 2019 at 16:35, Jingsong Li <jingsongl...@gmail.com> > >>> wrote: > >>>> > >>>>> Hi Jark, > >>>>> > >>>>> Let's recall FLIP-63, > >>>>> We supported these syntax in hive dialect at 1.9. All of my reasons > >> for > >>>>> launching FLIP-63 are to bring partition support to Flink itself. > >>>>> Not only batch, but also we have the need to stream jobs to write > >>> partition > >>>>> files today, which is also one of our very important application > >>> scenarios. > >>>>> > >>>>> The original intention of FLIP-63 is to bring all partition syntax to > >>>>> Flink, but in the end you and I have some different opinion in > >> creating > >>>>> partition table, so our consensus is to leave it in hive dialect, > only > >>> it. > >>>>> > >>>>> [1] > >>>>> > >>>>> > >>> > >> > https://cwiki.apache.org/confluence/display/FLINK/FLIP-63%3A+Rework+table+partition+support > >>>>> > >>>>> Best, > >>>>> Jingsong Lee > >>>>> > >>>>> On Thu, Dec 12, 2019 at 4:05 PM Jark Wu <imj...@gmail.com> wrote: > >>>>> > >>>>>> Hi jingsong, > >>>>>> > >>>>>> Watermark is not a standard syntax, that's why we had a FLIP and > long > >>>>>> discussion to add it to > >>>>>> Flink's SQL syntax. I think if we want to add INSERT OVERWRITE and > >>>>>> PARTITION syntax to > >>>>>> Flink's own syntax, we also need a FLIP or a VOTE, and this may > >>> can't > >>>>>> happen soon (we should > >>>>>> hear more people's opinions on this). > >>>>>> > >>>>>> Regarding to the sql-dialect configuration, I was not saying to > >> involve > >>>>> the > >>>>>> whole FLIP-89. I mean we can just > >>>>>> start a VOTE to expose it as `table.planner.sql-dialect` and include > >> it > >>>>> in > >>>>>> 1.10. The change can be very small, by > >>>>>> adding a ConfigOption and changing the implementation of > >>>>>> TableConfig#getSqlDialect/setSqlDialect. I believe > >>>>>> it is smaller and safer than changing the parser. > >>>>>> > >>>>>> Btw, I cc'ed Yu Li and Gary into the discussion, because release > >>> managers > >>>>>> should be aware of this. > >>>>>> > >>>>>> Best, > >>>>>> Jark > >>>>>> > >>>>>> > >>>>>> > >>>>>> On Thu, 12 Dec 2019 at 11:47, Danny Chan <yuzhao....@gmail.com> > >> wrote: > >>>>>> > >>>>>>> Thanks Jingsong for bring up this discussion ~ > >>>>>>> > >>>>>>> After reviewing FLIP-63, it seems that we have made a conclusion > for > >>>>> the > >>>>>>> syntax > >>>>>>> > >>>>>>> - INSERT OVERWRITE ... > >>>>>>> - INSERT INTO … PARTITION > >>>>>>> > >>>>>>> Which means that they should not have the Hive dialect limitation, > >> so > >>>>> I’m > >>>>>>> inclined that the behaviors for SQL-CLI is unexpected, or a “bug” > >> that > >>>>>> need > >>>>>>> to fix. > >>>>>>> > >>>>>>> We did not make a conclusion for the syntax: > >>>>>>> > >>>>>>> - CREATE TABLE … PARTITIONED BY ... > >>>>>>> > >>>>>>> Which means that the behavior of it is under-discussion, so it is > >> okey > >>>>> to > >>>>>>> be without the HIVE dialect limitation, we do not actually have any > >>>>> table > >>>>>>> sources/sinks that support such a DDL so for current code base, > >> users > >>>>>>> should not be influenced by the behaviors change. > >>>>>>> > >>>>>>> So I’m > >>>>>>> > >>>>>>> +1 to remove the hive dialect limitations for INSERT OVERWRITE and > >>>>> INSERT > >>>>>>> PARTITION > >>>>>>> +0 to add yaml dialect conf to SQL-CLI because FLIP-89 is not > >> finished > >>>>>>> yet, we better do this until FLIP-89 is resolved. > >>>>>>> > >>>>>>> Best, > >>>>>>> Danny Chan > >>>>>>> 在 2019年12月11日 +0800 PM5:29，Jingsong Li <jingsongl...@gmail.com > >，写道： > >>>>>>>> Hi Dev, > >>>>>>>> > >>>>>>>> After cutting out the branch of 1.10, I tried the following > >> functions > >>>>>> of > >>>>>>>> SQL-CLI and found that it does not support: > >>>>>>>> - insert overwrite > >>>>>>>> - PARTITION (partcol1=val1, partcol2=val2 ...) > >>>>>>>> The SQL pattern is: > >>>>>>>> INSERT { INTO | OVERWRITE } TABLE tablename1 [PARTITION > >>>>> (partcol1=val1, > >>>>>>>> partcol2=val2 ...) select_statement1 FROM from_statement; > >>>>>>>> It is a surprise to me. > >>>>>>>> The reason is that we only allow these two grammars in hive > >> dialect. > >>>>>> And > >>>>>>>> SQL-CLI does not have an interface to switch dialects. > >>>>>>>> > >>>>>>>> Because it directly hinders the SQL-CLI's insert syntax in hive > >>>>>>> integration > >>>>>>>> and seriously hinders the practicability of SQL-CLI. > >>>>>>>> And we have introduced these two grammars in FLIP-63 [1] to Flink. > >>>>>>>> Here are my question: > >>>>>>>> 1.Should we remove hive dialect limitation for these two grammars? > >>>>>>>> 2.Should we fix this in 1.10? > >>>>>>>> > >>>>>>>> [1] > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>> > >> > https://cwiki.apache.org/confluence/display/FLINK/FLIP-63%3A+Rework+table+partition+support > >>>>>>>> > >>>>>>>> Best, > >>>>>>>> Jingsong Lee > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> Best, Jingsong Lee > >>>>> > >>>> > >>> > >>> > >> > >> -- > >> Best, Jingsong Lee > >> > > > > > > -- Best, Jingsong Lee
