Hi Yubin,
Thanks for the quick response. The suggestion sounds good to me!
Best,
Jark
On Mon, 18 Mar 2024 at 13:06, Yubin Li <lyb5...@gmail.com> wrote:
> Hi Jark,
>
> Good pointing! Thanks for your reply, there are some details to align :)
>
> 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])
>
> Adopting { DESC | DESCRIBE } CATALOG [ EXTENDED ] xx as formal syntax,
> Producing rich and compatible results for future needs is very important.
> When
> specifying "extended" in the syntax, it will output the complete
> information including
> properties.The complete output example is as follows:
>
> +---------------------------------+---------------------------------------------------+
> | catalog_description_item | catalog_description_value |
>
> +---------------------------------+---------------------------------------------------+
> | Name | cat1
> |
> | Type | generic_in_memory
> |
> | Comment |
> |
> | Properties | ((k1,v1), (k2,v2))
> |
>
> +---------------------------------+---------------------------------------------------+
>
> 2. Could you add support for ALTER CATALOG xxx UNSET ('mykey')? This is
> > also very useful in ALTER TABLE.
>
> I found that there is already an ALTER TABLE xxx RESET ('mykey') syntax [1]
> now,
> which will reset the myKey attribute of a certain table to the default
> value. For catalogs,
> it might be better to use ALTER CATALOG xxx RESET ('mykey') for the sake of
> design
> consistency.
>
> WDYT? Looking forward to your suggestions.
>
> Best,
> Yubin
>
> [1]
>
> https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/alter/#reset
>
>
> On Mon, Mar 18, 2024 at 11:49 AM Jark Wu <imj...@gmail.com> wrote:
>
> > 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
> > > > >>
> > > > >>
> > > >
> > >
> >
>
