Hi, Yubin. Big +1 for this Flip. I just left one minor comment following.

I found that although flink has not supported syntax 'DESCRIBE CATALOG 
catalog_name' currently, it was already
discussed in flip-69[1], do we need to restart discussing it?
I don't have a particular preference regarding the restart discussion. It seems 
that there is no difference on this syntax 
in FLIP-436, so maybe it would be best to refer back to FLIP-69 in this FLIP. 
WDYT?


[1] 
https://cwiki.apache.org/confluence/display/FLINK/FLIP-69%3A+Flink+SQL+DDL+Enhancement



--

    Best!
    Xuyang





At 2024-03-15 02:49:59, "Yubin Li" <lyb5...@gmail.com> wrote:
>Hi folks,
>
>Thank you all for your input, it really makes sense to introduce missing
>catalog-related SQL syntaxes under this FLIP, and I have changed the
>title of doc to "FLIP-436: Introduce Catalog-related Syntax".
>
>After comprehensive consideration, the following syntaxes should be
>introduced, more suggestions are welcome :)
>
>> 1. SHOW CREATE CATALOG catalog_name
>> 2. DESCRIBE/DESC CATALOG catalog_name
>> 3. ALTER CATALOG catalog_name SET (key1=val1, key2=val2, ...)
>
>Regarding the `alter catalog` syntax format, I refer to the current design
>of `alter database`.
>
>Given that CatalogManager already provides catalog operations such as
>create, get, and unregister, and in order to facilitate future
>implementation
>of audit tracking, I propose to introduce the alterCatalog() function in
>CatalogManager. WDYT?
>
>Please see details in FLIP doc [1] .
>
>Best,
>Yubin
>
>[1]
>https://cwiki.apache.org/confluence/display/FLINK/FLIP-436%3A+Introduce+Catalog-related+Syntax
>
>
>On Thu, Mar 14, 2024 at 11:07 PM Leonard Xu <xbjt...@gmail.com> wrote:
>
>> Hi Yubin,
>>
>> Thanks for driving the discussion, generally +1 for the FLIP, big +1 to
>> finalize the whole catalog syntax story in one FLIP,
>> thus I want to jump into the discussion again after you completed the
>> whole catalog syntax story.
>>
>> Best,
>> Leonard
>>
>>
>>
>> > 2024年3月14日 下午8:39,Roc Marshal <flin...@126.com> 写道:
>> >
>> > Hi, Yubin
>> >
>> >
>> > Thank you for initiating this discussion! +1 for the proposal.
>> >
>> >
>> >
>> >
>> >
>> >
>> > Best,
>> > Yuepeng Pan
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > At 2024-03-14 18:57:35, "Ferenc Csaky" <ferenc.cs...@pm.me.INVALID>
>> wrote:
>> >> Hi Yubin,
>> >>
>> >> Thank you for initiating this discussion! +1 for the proposal.
>> >>
>> >> I also think it makes sense to group the missing catalog related
>> >> SQL syntaxes under this FLIP.
>> >>
>> >> Looking forward to these features!
>> >>
>> >> Best,
>> >> Ferenc
>> >>
>> >>
>> >>
>> >>
>> >> On Thursday, March 14th, 2024 at 08:31, Jane Chan <
>> qingyue....@gmail.com> wrote:
>> >>
>> >>>
>> >>>
>> >>> Hi Yubin,
>> >>>
>> >>> Thanks for leading the discussion. I'm +1 for the FLIP.
>> >>>
>> >>> As Jark said, it's a good opportunity to enhance the syntax for Catalog
>> >>> from a more comprehensive perspective. So, I suggest expanding the
>> scope of
>> >>> this FLIP by focusing on the mechanism instead of one use case to
>> enhance
>> >>> the overall functionality. WDYT?
>> >>>
>> >>> Best,
>> >>> Jane
>> >>>
>> >>> On Thu, Mar 14, 2024 at 11:38 AM Hang Ruan ruanhang1...@gmail.com
>> wrote:
>> >>>
>> >>>> Hi, Yubin.
>> >>>>
>> >>>> Thanks for the FLIP. +1 for it.
>> >>>>
>> >>>> Best,
>> >>>> Hang
>> >>>>
>> >>>> Yubin Li lyb5...@gmail.com 于2024年3月14日周四 10:15写道:
>> >>>>
>> >>>>> Hi Jingsong, Feng, and Jeyhun
>> >>>>>
>> >>>>> Thanks for your support and feedback!
>> >>>>>
>> >>>>>> However, could we add a new method `getCatalogDescriptor()` to
>> >>>>>> CatalogManager instead of directly exposing CatalogStore?
>> >>>>>
>> >>>>> Good point, Besides the audit tracking issue, The proposed feature
>> >>>>> only requires `getCatalogDescriptor()` function. Exposing components
>> >>>>> with excessive functionality will bring unnecessary risks, I have
>> made
>> >>>>> modifications in the FLIP doc [1]. Thank Feng :)
>> >>>>>
>> >>>>>> Showing the SQL parser implementation in the FLIP for the SQL syntax
>> >>>>>> might be a bit confusing. Also, the formal definition is missing for
>> >>>>>> this SQL clause.
>> >>>>>
>> >>>>> Thank Jeyhun for pointing it out :) I have updated the doc [1] .
>> >>>>>
>> >>>>> [1]
>> >>>>
>> >>>>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=296290756
>> >>>>
>> >>>>> Best,
>> >>>>> Yubin
>> >>>>>
>> >>>>> On Thu, Mar 14, 2024 at 2:18 AM Jeyhun Karimov je.kari...@gmail.com
>> >>>>> wrote:
>> >>>>>
>> >>>>>> Hi Yubin,
>> >>>>>>
>> >>>>>> Thanks for the proposal. +1 for it.
>> >>>>>> I have one comment:
>> >>>>>>
>> >>>>>> I would like to see the SQL syntax for the proposed statement.
>> Showing
>> >>>>>> the
>> >>>>>> SQL parser implementation in the FLIP
>> >>>>>> for the SQL syntax might be a bit confusing. Also, the formal
>> >>>>>> definition
>> >>>>>> is
>> >>>>>> missing for this SQL clause.
>> >>>>>> Maybe something like [1] might be useful. WDYT?
>> >>>>>>
>> >>>>>> Regards,
>> >>>>>> Jeyhun
>> >>>>>>
>> >>>>>> [1]
>> >>>>
>> >>>>
>> https://github.com/apache/flink/blob/0da60ca1a4754f858cf7c52dd4f0c97ae0e1b0cb/docs/content/docs/dev/table/sql/show.md?plain=1#L620-L632
>> >>>>
>> >>>>>> On Wed, Mar 13, 2024 at 3:28 PM Feng Jin jinfeng1...@gmail.com
>> >>>>>> wrote:
>> >>>>>>
>> >>>>>>> Hi Yubin
>> >>>>>>>
>> >>>>>>> Thank you for initiating this FLIP.
>> >>>>>>>
>> >>>>>>> I have just one minor question:
>> >>>>>>>
>> >>>>>>> I noticed that we added a new function `getCatalogStore` to expose
>> >>>>>>> CatalogStore, and it seems fine.
>> >>>>>>> However, could we add a new method `getCatalogDescriptor()` to
>> >>>>>>> CatalogManager instead of directly exposing CatalogStore?
>> >>>>>>> By only providing the `getCatalogDescriptor()` interface, it may be
>> >>>>>>> easier
>> >>>>>>> for us to implement audit tracking in CatalogManager in the future.
>> >>>>>>> WDYT ?
>> >>>>>>> Although we have only collected some modified events at the
>> >>>>>>> moment.[1]
>> >>>>>>>
>> >>>>>>> [1].
>> >>>>
>> >>>>
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-294%3A+Support+Customized+Catalog+Modification+Listener
>> >>>>
>> >>>>>>> Best,
>> >>>>>>> Feng
>> >>>>>>>
>> >>>>>>> On Wed, Mar 13, 2024 at 5:31 PM Jingsong Li jingsongl...@gmail.com
>> >>>>>>> wrote:
>> >>>>>>>
>> >>>>>>>> +1 for this.
>> >>>>>>>>
>> >>>>>>>> We are missing a series of catalog related syntaxes.
>> >>>>>>>> Especially after the introduction of catalog store. [1]
>> >>>>>>>>
>> >>>>>>>> [1]
>> >>>>
>> >>>>
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-295%3A+Support+lazy+initialization+of+catalogs+and+persistence+of+catalog+configurations
>> >>>>
>> >>>>>>>> Best,
>> >>>>>>>> Jingsong
>> >>>>>>>>
>> >>>>>>>> On Wed, Mar 13, 2024 at 5:09 PM Yubin Li lyb5...@gmail.com
>> >>>>>>>> wrote:
>> >>>>>>>>
>> >>>>>>>>> Hi devs,
>> >>>>>>>>>
>> >>>>>>>>> I'd like to start a discussion about FLIP-436: Introduce "SHOW
>> >>>>>>>>> CREATE
>> >>>>>>>>> CATALOG" Syntax [1].
>> >>>>>>>>>
>> >>>>>>>>> At present, the `SHOW CREATE TABLE` statement provides strong
>> >>>>>>>>> support
>> >>>>>>>>> for
>> >>>>>>>>> users to easily
>> >>>>>>>>> reuse created tables. However, despite the increasing importance
>> >>>>>>>>> of the
>> >>>>>>>>> `Catalog` in user's
>> >>>>>>>>> business, there is no similar statement for users to use.
>> >>>>>>>>>
>> >>>>>>>>> According to the online discussion in FLINK-24939 [2] with Jark
>> >>>>>>>>> Wu
>> >>>>>>>>> and
>> >>>>>>>>> Feng
>> >>>>>>>>> Jin, since `CatalogStore`
>> >>>>>>>>> has been introduced in FLIP-295 [3], we could use this component
>> >>>>>>>>> to
>> >>>>>>>>> implement such a long-awaited
>> >>>>>>>>> feature, Please refer to the document [1] for implementation
>> >>>>>>>>> details.
>> >>>>>>>>>
>> >>>>>>>>> examples as follows:
>> >>>>>>>>>
>> >>>>>>>>> Flink SQL> create catalog cat2 WITH ('type'='generic_in_memory',
>> >>>>>>>>>
>> >>>>>>>>>> 'default-database'='db');
>> >>>>>>>>>> [INFO] Execute statement succeeded.
>> >>>>>>>>>> Flink SQL> show create catalog cat2;
>> >>>>
>> >>>>
>> +----------------------------------------------------------------------------------------+
>> >>>>
>> >>>>>>>>>> | result |
>> >>>>
>> >>>>
>> +----------------------------------------------------------------------------------------+
>> >>>>
>> >>>>>>>>>> | CREATE CATALOG `cat2` WITH (
>> >>>>>>>>>> 'default-database' = 'db',
>> >>>>>>>>>> 'type' = 'generic_in_memory'
>> >>>>>>>>>> )
>> >>>>>>>>>> |
>> >>>>
>> >>>>
>> +----------------------------------------------------------------------------------------+
>> >>>>
>> >>>>>>>>>> 1 row in set
>> >>>>>>>>>
>> >>>>>>>>> Looking forward to hearing from you, thanks!
>> >>>>>>>>>
>> >>>>>>>>> Best regards,
>> >>>>>>>>> Yubin
>> >>>>>>>>>
>> >>>>>>>>> [1]
>> >>>>
>> >>>>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=296290756
>> >>>>
>> >>>>>>>>> [2] https://issues.apache.org/jira/browse/FLINK-24939
>> >>>>>>>>> [3]
>> >>>>
>> >>>>
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-295%3A+Support+lazy+initialization+of+catalogs+and+persistence+of+catalog+configurations
>>
>>

Reply via email to