Hi Yubin,
Thanks for updating the FLIP. The updated version looks good in general.
I only have 2 minor comments.
1. I think the purpose of DESCRIBE CATALOG is to display metadata
information including catalog name,
catalog comment (may be introduced in the future), catalog type, and
catalog properties (for example [1]).
Expanding all properties may limit this syntax to include more metadata
information in the future.
2. Could you add support for ALTER CATALOG xxx UNSET ('mykey')? This is
also very useful in ALTER TABLE.
Best,
Jark
[1]:
https://docs.databricks.com/en/sql/language-manual/sql-ref-syntax-aux-describe-schema.html
On Fri, 15 Mar 2024 at 12:06, Yubin Li <lyb5...@gmail.com> wrote:
> Hi Xuyang,
>
> Thank you for pointing this out, The parser part of `describe catalog`
> syntax
> has indeed been implemented in FLIP-69, and it is not actually available.
> we can complete the syntax in this FLIP [1]. I have updated the doc :)
>
> Best,
> Yubin
>
> [1]
>
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-436%3A+Introduce+Catalog-related+Syntax
>
> On Fri, Mar 15, 2024 at 10:12 AM Xuyang <xyzhong...@163.com> wrote:
>
> > 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
> > >>
> > >>
> >
>
