Thanks Jingsong.
> Just naming, maybe `createCatalog` in TableEnv
+1 For this, I have already updated FLIP.
Best,
Feng
On Fri, Jun 2, 2023 at 11:32 AM Jingsong Li <jingsongl...@gmail.com> wrote:
> Thanks Feng,
>
> Just naming, maybe `createCatalog` in TableEnv, I can see many
> functions are converted to createxxx from registerxxx.
>
> On Fri, Jun 2, 2023 at 11:04 AM Feng Jin <jinfeng1...@gmail.com> wrote:
> >
> > Hi jark, thanks for your suggestion.
> >
> > > 1. How to register the CatalogStore for Table API? I think the
> > CatalogStore should be immutable once TableEnv is created. Otherwise,
> there
> > might be some data inconsistencies when CatalogStore is changed.
> >
> > Yes, We should initialize the CatalogStore when creating the TableEnv.
> > Therefore, my current proposal is to add a method to configure the
> > CatalogStore in EnvironmentSettings.
> >
> > // Initialize a catalog Store
> > CatalogStore catalogStore = new FileCatalogStore("");
> >
> > // set up the Table API
> > final EnvironmentSettings settings =
> > EnvironmentSettings.newInstance().inBatchMode()
> > .withCatalogStore(catalogStore)
> > .build();
> >
> > final TableEnvironment tableEnv =
> TableEnvironment.create(settings);
> >
> >
> > > 2. Why does the CatalogStoreFactory interface only have a default
> method,
> > not an interface method?
> >
> > Sorry, While I did refer to the Catalog interface, I agree that as a new
> > interface, the CatalogStoreFactory should not have a default method but
> an
> > interface method. I have already modified the interface.
> >
> > > 3. Please mention the alternative API in Javadoc for the
> > deprecated`registerCatalog`.
> > > 4. In the "Compatibility" section, would be better to mention the
> changed
> > behavior of CREATE CATALOG statement if FileCatalogStore (or other
> > persisted catalog store) is used.
> >
> > Thanks for the suggestion, I have updated the FLIP [1].
> >
> >
> > [1].
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-295%3A+Support+lazy+initialization+of+catalogs+and+persistence+of+catalog+configurations
> >
> >
> > Best,
> > Feng
> >
> > On Thu, Jun 1, 2023 at 9:22 PM Jark Wu <imj...@gmail.com> wrote:
> >
> > > Hi Feng,
> > >
> > > This is a useful FLIP. Thanks for starting this discussion.
> > > The current design looks pretty good to me. I just have some minor
> > > comments.
> > >
> > > 1. How to register the CatalogStore for Table API? I think the
> CatalogStore
> > > should be immutable once TableEnv is created. Otherwise, there might be
> > > some data inconsistencies when CatalogStore is changed.
> > >
> > > 2. Why does the CatalogStoreFactory interface only have a default
> method,
> > > not an interface method?
> > >
> > > 3. Please mention the alternative API in Javadoc for the deprecated
> > > `registerCatalog`.
> > >
> > > 4. In the "Compatibility" section, would be better to mention the
> changed
> > > behavior of CREATE CATALOG statement if FileCatalogStore (or other
> > > persisted catalog store) is used.
> > >
> > >
> > > Best,
> > > Jark
> > >
> > > On Thu, 1 Jun 2023 at 11:26, Feng Jin <jinfeng1...@gmail.com> wrote:
> > >
> > > > Hi ， thanks all for reviewing the flip.
> > > >
> > > > @Ron
> > > >
> > > > > Regarding the CatalogStoreFactory#createCatalogStore method, do we
> > > need
> > > > to provide a default implementation?
> > > >
> > > > Yes, we will provide a default InMemoryCatalogStoreFactory to create
> an
> > > > InMemoryCatalogStore.
> > > >
> > > > > If we get a Catalog from CatalogStore, after initializing it,
> whether
> > > we
> > > > put it in Map<String, Catalog> catalogs again?
> > > >
> > > > Yes, in the current design, catalogs are stored as snapshots, and
> once
> > > > initialized, the Catalog will be placed in the Map<String, Catalog>
> > > > catalogs.
> > > > Subsequently, the Map<String, Catalog> catalogs will be the primary
> > > source
> > > > for obtaining the corresponding Catalog.
> > > >
> > > > > how about renaming them to `catalog.store.type` and
> > > > `catalog.store.path`?
> > > >
> > > > I think it is okay. Adding "sql" at the beginning may seem a bit
> > > strange. I
> > > > will update the FLIP.
> > > >
> > > >
> > > >
> > > > @Shammon
> > > >
> > > > Thank you for the review. I have made the necessary corrections.
> > > > Regarding the modifications made to the Public Interface, I have also
> > > > included the relevant changes to the `TableEnvironment`.
> > > >
> > > >
> > > > Best,
> > > > Feng
> > > >
> > > >
> > > > On Wed, May 31, 2023 at 5:02 PM Shammon FY <zjur...@gmail.com>
> wrote:
> > > >
> > > > > Hi feng,
> > > > >
> > > > > Thanks for updating, I have some minor comments
> > > > >
> > > > > 1. The modification of `CatalogManager` should not be in `Public
> > > > > Interfaces`, it is not a public interface.
> > > > >
> > > > > 2. `@PublicEvolving` should be added for `CatalogStore` and
> > > > > `CatalogStoreFactory`
> > > > >
> > > > > 3. The code `Optional<Catalog> optionalDescriptor =
> > > > > catalogStore.get(catalogName);` in the `CatalogManager` should be
> > > > > `Optional<CatalogDescriptor> optionalDescriptor =
> > > > > catalogStore.get(catalogName);`
> > > > >
> > > > > Best,
> > > > > Shammon FY
> > > > >
> > > > >
> > > > > On Wed, May 31, 2023 at 2:24 PM liu ron <ron9....@gmail.com>
> wrote:
> > > > >
> > > > > > Hi, Feng
> > > > > >
> > > > > > Thanks for driving this FLIP, this proposal is very useful for
> > > catalog
> > > > > > management.
> > > > > > I have some small questions:
> > > > > >
> > > > > > 1. Regarding the CatalogStoreFactory#createCatalogStore method,
> do we
> > > > > need
> > > > > > to provide a default implementation?
> > > > > > 2. If we get Catalog from CatalogStore, after initializing it,
> > > whether
> > > > we
> > > > > > put it to Map<String, Catalog> catalogs again?
> > > > > > 3. Regarding the options `sql.catalog.store.type` and
> > > > > > `sql.catalog.store.file.path`, how about renaming them to
> > > > > > `catalog.store.type` and `catalog.store.path`?
> > > > > >
> > > > > > Best,
> > > > > > Ron
> > > > > >
> > > > > > Feng Jin <jinfeng1...@gmail.com> 于2023年5月29日周一 21:19写道：
> > > > > >
> > > > > > > Hi yuxia
> > > > > > >
> > > > > > > > But from the code in Proposed Changes, once we register the
> > > > Catalog,
> > > > > > we
> > > > > > > initialize it and open it. right?
> > > > > > >
> > > > > > > Yes, In order to avoid inconsistent semantics of the original
> > > CREATE
> > > > > > > CATALOG DDL, Catalog will be directly initialized in
> > > registerCatalog
> > > > so
> > > > > > > that parameter validation can be performed.
> > > > > > >
> > > > > > > In the current design, lazy initialization is mainly reflected
> in
> > > > > > > getCatalog. If CatalogStore has already saved some catalog
> > > > > > configurations,
> > > > > > > only initialization is required in getCatalog.
> > > > > > >
> > > > > > >
> > > > > > > Best,
> > > > > > > Feng
> > > > > > >
> > > > > > > On Mon, May 29, 2023 at 8:27 PM yuxia <
> luoyu...@alumni.sjtu.edu.cn
> > > >
> > > > > > wrote:
> > > > > > >
> > > > > > > > Hi, Feng.
> > > > > > > > I'm trying to understanding the meaning of *lazy
> initialization*.
> > > > If
> > > > > > i'm
> > > > > > > > wrong, please correct me.
> > > > > > > >
> > > > > > > > IIUC, lazy initialization means only you need to access the
> > > > catalog,
> > > > > > then
> > > > > > > > you initialize it. But from the code in Proposed Changes,
> once we
> > > > > > > register
> > > > > > > > the Catalog,
> > > > > > > > we initialize it and open it. right?
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Yuxia
> > > > > > > >
> > > > > > > > ----- 原始邮件 -----
> > > > > > > > 发件人: "Jing Ge" <j...@ververica.com.INVALID>
> > > > > > > > 收件人: "dev" <dev@flink.apache.org>
> > > > > > > > 发送时间: 星期一, 2023年 5 月 29日 下午 5:12:46
> > > > > > > > 主题: Re: [DISCUSS] FLIP 295: Support persistence of Catalog
> > > > > > configuration
> > > > > > > > and asynchronous registration
> > > > > > > >
> > > > > > > > Hi Feng,
> > > > > > > >
> > > > > > > > Thanks for your effort! +1 for the proposal.
> > > > > > > >
> > > > > > > > One of the major changes is that current design will provide
> > > > > > > > Map<String,Catalog> catalogs as a snapshot instead of a
> cache,
> > > > which
> > > > > > > means
> > > > > > > > once it has been initialized, any changes done by other
> sessions
> > > > will
> > > > > > not
> > > > > > > > affect it. Point 6 described follow-up options for further
> > > > > improvement.
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Jing
> > > > > > > >
> > > > > > > > On Mon, May 29, 2023 at 5:31 AM Feng Jin <
> jinfeng1...@gmail.com>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi all, I would like to update you on the latest progress
> of
> > > the
> > > > > > FLIP.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Last week, Leonard Xu, HangRuan, Jing Ge, Shammon FY,
> ShengKai
> > > > Fang
> > > > > > > and I
> > > > > > > > > had an offline discussion regarding the overall solution
> for
> > > > Flink
> > > > > > > > > CatalogStore. We have reached a consensus and I have
> updated
> > > the
> > > > > > final
> > > > > > > > > solution in FLIP.
> > > > > > > > >
> > > > > > > > > Next, let me briefly describe the entire design:
> > > > > > > > >
> > > > > > > > > 1.
> > > > > > > > >
> > > > > > > > > Introduce CatalogDescriptor to store catalog
> configuration
> > > > > similar
> > > > > > > to
> > > > > > > > > TableDescriptor.
> > > > > > > > > 2.
> > > > > > > > >
> > > > > > > > > The two key functions of CatalogStore - void
> > > > storeCatalog(String
> > > > > > > > > catalogName, CatalogDescriptor) and CatalogDescriptor
> > > > > > > > getCatalog(String)
> > > > > > > > > will both use CatalogDescriptor instead of Catalog
> instance.
> > > > > This
> > > > > > > way,
> > > > > > > > > CatalogStore will only be responsible for saving and
> > > > retrieving
> > > > > > > > catalog
> > > > > > > > > configurations without having to initialize catalogs.
> > > > > > > > > 3.
> > > > > > > > >
> > > > > > > > > The default registerCatalog(String catalogName, Catalog
> > > > catalog)
> > > > > > > > > function in CatalogManager will be marked as deprecated.
> > > > > > > > > 4.
> > > > > > > > >
> > > > > > > > > A new function registerCatalog(String catalogName,
> > > > > > CatalogDescriptor
> > > > > > > > > catalog) will be added to serve as the default
> registration
> > > > > > function
> > > > > > > > for
> > > > > > > > > catalogs in CatalogManager.
> > > > > > > > > 5.
> > > > > > > > >
> > > > > > > > > Map<String,Catalog> catalogs in CataloManager will
> remain
> > > > > > unchanged
> > > > > > > > and
> > > > > > > > > save initialized catalogs.This means that deletion
> > > operations
> > > > > from
> > > > > > > one
> > > > > > > > > session won't synchronize with other sessions.
> > > > > > > > > 6.
> > > > > > > > >
> > > > > > > > > To support multi-session synchronization scenarios for
> > > > deletions
> > > > > > > later
> > > > > > > > > on we should make Map<String,Catalog>catalogs
> > > > configurable.There
> > > > > > may
> > > > > > > > be
> > > > > > > > > three possible situations:
> > > > > > > > >
> > > > > > > > > a.Default caching of all initialized catalogs
> > > > > > > > >
> > > > > > > > > b.Introduction of LRU cache logic which can
> automatically
> > > > clear
> > > > > > > > > long-unused catalogs.
> > > > > > > > >
> > > > > > > > > c.No caching of any instances; each call to getCatalog
> > > > creates a
> > > > > > new
> > > > > > > > > instance.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > This is the document for discussion:
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> https://docs.google.com/document/d/1HRJNd4_id7i6cUxGnAybmYZIwl5g1SmZCOzGdUz-6lU/edit
> > > > > > > > >
> > > > > > > > > This is the final proposal document:
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-295%3A+Support+lazy+initialization+of+catalogs+and+persistence+of+catalog+configurations
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Thank you very much for your attention and suggestions on
> this
> > > > > > FLIP. A
> > > > > > > > > special thanks to Hang Ruan for his suggestions on the
> entire
> > > > > design
> > > > > > > and
> > > > > > > > > organizing offline discussions.
> > > > > > > > >
> > > > > > > > > If you have any further suggestions or feedback about this
> FLIP
> > > > > > please
> > > > > > > > feel
> > > > > > > > > free to share.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > >
> > > > > > > > > Feng
> > > > > > > > >
> > > > > > > > > On Sat, May 6, 2023 at 8:32 PM Jing Ge
> > > > <j...@ververica.com.invalid
> > > > > >
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Feng,
> > > > > > > > > >
> > > > > > > > > > Thanks for improving the FLIP. It looks good to me. We
> could
> > > > > still
> > > > > > > > > > reconsider in the future how to provide more common
> built-in
> > > > > cache
> > > > > > > > > > functionality in CatalogManager so that not every
> > > CatalogSotre
> > > > > > > > > > implementation has to take care of it.
> > > > > > > > > >
> > > > > > > > > > Best regards,
> > > > > > > > > > Jing
> > > > > > > > > >
> > > > > > > > > > On Thu, May 4, 2023 at 1:47 PM Feng Jin <
> > > jinfeng1...@gmail.com
> > > > >
> > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi Jing,
> > > > > > > > > > >
> > > > > > > > > > > Thanks for your reply.
> > > > > > > > > > >
> > > > > > > > > > > > There might be more such issues. I would suggest you
> > > > > > completely
> > > > > > > > walk
> > > > > > > > > > > through the FLIP again and fix those issues
> > > > > > > > > > >
> > > > > > > > > > > I am very sorry for my carelessness and at the same
> time, I
> > > > > > greatly
> > > > > > > > > > > appreciate your careful review. I have thoroughly
> checked
> > > the
> > > > > > > entire
> > > > > > > > > FLIP
> > > > > > > > > > > and made corrections to these issues.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > If I am not mistaken, with the current FLIP design,
> > > > > > > > CatalogManager
> > > > > > > > > > > could work without Optional CatalogStore being
> configured.
> > > > > > > > > > >
> > > > > > > > > > > Yes, in the original design, CatalogStore was not
> necessary
> > > > > > because
> > > > > > > > > > > CatalogManager used Map<String, Catalog> catalogs to
> store
> > > > > > catalog
> > > > > > > > > > > instances.
> > > > > > > > > > > However, this caused inconsistency issues. Therefore, I
> > > > > modified
> > > > > > > this
> > > > > > > > > > part
> > > > > > > > > > > of the design and removed Map<String, Catalog> catalogs
> > > from
> > > > > > > > > > > CatalogManager.
> > > > > > > > > > > At the same time, InMemoryCatalog will serve as the
> > > default
> > > > > > > > > CatalogStore
> > > > > > > > > > > to save catalogs in memory and replace the
> functionality of
> > > > > > > > > > > Map<String,Catalog>catalogs.
> > > > > > > > > > > The previous plan that kept
> Map<String,Catalog>catalogs has
> > > > > been
> > > > > > > > moved
> > > > > > > > > to
> > > > > > > > > > > Rejected Alternatives.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Best,
> > > > > > > > > > > Feng
> > > > > > > > > > >
> > > > > > > > > > > On Sun, Apr 30, 2023 at 9:03 PM Jing Ge
> > > > > > <j...@ververica.com.invalid
> > > > > > > >
> > > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi Feng,
> > > > > > > > > > > >
> > > > > > > > > > > > There are still many places contain inconsistent
> content,
> > > > > e.g.
> > > > > > > > > > > >
> > > > > > > > > > > > 1. "Asynchronous registration" is still used.
> > > > > > > > > > > > 2. The java comment of the method
> registerCatalog(String
> > > > > > > > catalogName,
> > > > > > > > > > > > Catalog catalog) in CatalogManager does not tell
> what the
> > > > > > method
> > > > > > > is
> > > > > > > > > > > doing.
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > There might be more such issues. I would suggest you
> > > > > completely
> > > > > > > > walk
> > > > > > > > > > > > through the FLIP again and fix those issues.
> > > > > > > > > > > >
> > > > > > > > > > > > About the inMemoryCatalogStore, do you mean that you
> will
> > > > > build
> > > > > > > the
> > > > > > > > > > cache
> > > > > > > > > > > > functionality in the CatalogStore? This is a very
> > > different
> > > > > > > design
> > > > > > > > > > > concept
> > > > > > > > > > > > from what the current FLIP described. If I am not
> > > mistaken,
> > > > > > with
> > > > > > > > the
> > > > > > > > > > > > current FLIP design, CatalogManager could work
> without
> > > > > Optional
> > > > > > > > > > > > CatalogStore being configured. That is the reason
> why I
> > > > > > mentioned
> > > > > > > > in
> > > > > > > > > > the
> > > > > > > > > > > > last email that the example code wrt the Optional
> > > > > CatalogStore
> > > > > > is
> > > > > > > > not
> > > > > > > > > > > > correct.
> > > > > > > > > > > >
> > > > > > > > > > > > Best regards,
> > > > > > > > > > > > Jing
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, Apr 27, 2023 at 3:55 PM Feng Jin <
> > > > > > jinfeng1...@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Hi Jing
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > There are still some NIT issues in the FLIP
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thank you very much for the careful review. I have
> > > > already
> > > > > > made
> > > > > > > > the
> > > > > > > > > > > > > relevant changes.
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Speaking of the conflict issues in the
> > > multi-instance
> > > > > > > > > scenarios, I
> > > > > > > > > > > am
> > > > > > > > > > > > > not
> > > > > > > > > > > > > sure if this is the intended behaviour
> > > > > > > > > > > > >
> > > > > > > > > > > > > Currently, there are conflicts in multiple
> scenarios
> > > with
> > > > > the
> > > > > > > > > current
> > > > > > > > > > > > > design. I am thinking whether we should remove
> > > > 'Map<String,
> > > > > > > > > Catalog>'
> > > > > > > > > > > and
> > > > > > > > > > > > > make Cache the default behavior of
> > > InMemoryCatalogStore.
> > > > > This
> > > > > > > > way,
> > > > > > > > > > > users
> > > > > > > > > > > > > can implement their own CatalogStore to achieve
> > > > > > multi-instance
> > > > > > > > > > > > > non-conflicting scenarios. What do you think?
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > Best,
> > > > > > > > > > > > > Feng
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Thu, Apr 27, 2023 at 9:03 PM Jing Ge
> > > > > > > > <j...@ververica.com.invalid
> > > > > > > > > >
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Feng,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Thanks for working on the FLIP. There are still
> some
> > > > NIT
> > > > > > > issues
> > > > > > > > > in
> > > > > > > > > > > the
> > > > > > > > > > > > > FLIP
> > > > > > > > > > > > > > like:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > 1. Optional<CatalogStore> catalogStore has been
> used
> > > as
> > > > > > > > > > CatalogStore
> > > > > > > > > > > > > > instead of Optional in the code example. It
> should be
> > > > > fine
> > > > > > to
> > > > > > > > use
> > > > > > > > > > it
> > > > > > > > > > > as
> > > > > > > > > > > > > > pseudo code for now and update it after you
> submit
> > > the
> > > > > PR.
> > > > > > > > > > > > > > 2. addCatalog(...) is still used somewhere in the
> > > > > rejected
> > > > > > > > > section
> > > > > > > > > > > > which
> > > > > > > > > > > > > > should be persistContext(...) to keep it
> consistent.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Speaking of the conflict issues in the
> multi-instance
> > > > > > > > scenarios,
> > > > > > > > > I
> > > > > > > > > > am
> > > > > > > > > > > > not
> > > > > > > > > > > > > > sure if this is the intended behaviour. If
> > > Map<String,
> > > > > > > Catalog>
> > > > > > > > > > > > catalogs
> > > > > > > > > > > > > is
> > > > > > > > > > > > > > used as a cache, it should be invalid, once the
> > > related
> > > > > > > catalog
> > > > > > > > > has
> > > > > > > > > > > > been
> > > > > > > > > > > > > > removed from the CatalogStore by another
> instance.
> > > Did
> > > > I
> > > > > > miss
> > > > > > > > > > > > something?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Best regards,
> > > > > > > > > > > > > > Jing
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Thu, Apr 13, 2023 at 4:40 PM Feng Jin <
> > > > > > > > jinfeng1...@gmail.com>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Hi Jing,Shammon
> > > > > > > > > > > > > > > Thanks for your reply.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > @Jing
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > How about persistCatalog()?
> > > > > > > > > > > > > > > I think this is a good function name, I have
> > > updated
> > > > it
> > > > > > in
> > > > > > > > the
> > > > > > > > > > > > > > > documentation.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Some common cache features should be
> > > implemented
> > > > > > > > > > > > > > > Thank you for the suggestion. If alternative 1
> > > turns
> > > > > out
> > > > > > to
> > > > > > > > be
> > > > > > > > > > more
> > > > > > > > > > > > > > > appropriate later, I will improve this part of
> the
> > > > > > content.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > As the above discussion moves forward, the
> > > option 2
> > > > > > > > solution
> > > > > > > > > > > looks
> > > > > > > > > > > > > more
> > > > > > > > > > > > > > > like a replacement of option 1
> > > > > > > > > > > > > > > Yes, after discussing with Shammon offline, we
> > > think
> > > > > that
> > > > > > > > > > solution
> > > > > > > > > > > 2
> > > > > > > > > > > > > > might
> > > > > > > > > > > > > > > be more suitable and also avoid any
> inconsistency
> > > > > issues.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > There are some inconsistent descriptions in
> the
> > > > > > content.
> > > > > > > > > Would
> > > > > > > > > > > you
> > > > > > > > > > > > > > like
> > > > > > > > > > > > > > > to clean them up?
> > > > > > > > > > > > > > > I will do my best to improve the document and
> > > > > appreciate
> > > > > > > your
> > > > > > > > > > > > > > suggestions.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > @Shammon
> > > > > > > > > > > > > > > > can you put the unselected option in
> `Rejected
> > > > > > > > Alternatives`
> > > > > > > > > > > > > > > Sure, I have moved it to the `Rejected
> > > Alternatives`.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Best
> > > > > > > > > > > > > > > Feng
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Thu, Apr 13, 2023 at 8:52 AM Shammon FY <
> > > > > > > > zjur...@gmail.com>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi Feng
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Thanks for your update.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I found there are two options in `Proposed
> > > > Changes`,
> > > > > > can
> > > > > > > > you
> > > > > > > > > > put
> > > > > > > > > > > > the
> > > > > > > > > > > > > > > > unselected option in `Rejected
> Alternatives`? I
> > > > think
> > > > > > > this
> > > > > > > > > may
> > > > > > > > > > > help
> > > > > > > > > > > > > us
> > > > > > > > > > > > > > > > better understand your proposal
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Best,
> > > > > > > > > > > > > > > > Shammon FY
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Thu, Apr 13, 2023 at 4:49 AM Jing Ge
> > > > > > > > > > > <j...@ververica.com.invalid
> > > > > > > > > > > > >
> > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Hi Feng,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Thanks for raising this FLIP. I am still
> > > confused
> > > > > > after
> > > > > > > > > > > > completely
> > > > > > > > > > > > > > > > reading
> > > > > > > > > > > > > > > > > the thread with following questions:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > 1. Naming confusion - registerCatalog() and
> > > > > > > addCatalog()
> > > > > > > > > have
> > > > > > > > > > > no
> > > > > > > > > > > > > big
> > > > > > > > > > > > > > > > > difference based on their names. One of
> them is
> > > > > > > > responsible
> > > > > > > > > > for
> > > > > > > > > > > > > data
> > > > > > > > > > > > > > > > > persistence. How about persistCatalog()?
> > > > > > > > > > > > > > > > > 2. As you mentioned that Map<String,
> Catalog>
> > > > > > catalogs
> > > > > > > is
> > > > > > > > > > used
> > > > > > > > > > > > as a
> > > > > > > > > > > > > > > cache
> > > > > > > > > > > > > > > > > and catalogStore is used for data
> persistence.
> > > I
> > > > > > would
> > > > > > > > > > suggest
> > > > > > > > > > > > > > > describing
> > > > > > > > > > > > > > > > > their purpose conceptually and clearly in
> the
> > > > FLIP.
> > > > > > > Some
> > > > > > > > > > common
> > > > > > > > > > > > > cache
> > > > > > > > > > > > > > > > > features should be implemented, i.e. data
> in
> > > the
> > > > > > cache
> > > > > > > > and
> > > > > > > > > > the
> > > > > > > > > > > > > store
> > > > > > > > > > > > > > > > should
> > > > > > > > > > > > > > > > > be consistent. Same Catalog instance
> should be
> > > > > found
> > > > > > in
> > > > > > > > the
> > > > > > > > > > > store
> > > > > > > > > > > > > and
> > > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > the cache(either it has been initialized
> or it
> > > > will
> > > > > > be
> > > > > > > > lazy
> > > > > > > > > > > > > > > initialized)
> > > > > > > > > > > > > > > > > for the same catalog name. The consistency
> will
> > > > be
> > > > > > > taken
> > > > > > > > > care
> > > > > > > > > > > of
> > > > > > > > > > > > > > while
> > > > > > > > > > > > > > > > > updating the catalog.
> > > > > > > > > > > > > > > > > 3. As the above discussion moves forward,
> the
> > > > > option
> > > > > > 2
> > > > > > > > > > solution
> > > > > > > > > > > > > looks
> > > > > > > > > > > > > > > > more
> > > > > > > > > > > > > > > > > like a replacement of option 1, because,
> afaiu,
> > > > > > issues
> > > > > > > > > > > mentioned
> > > > > > > > > > > > > > > > > previously with option 1 are not solved
> yet. Do
> > > > you
> > > > > > > still
> > > > > > > > > > want
> > > > > > > > > > > to
> > > > > > > > > > > > > > > propose
> > > > > > > > > > > > > > > > > both options and ask for suggestions for
> both
> > > of
> > > > > > them?
> > > > > > > > > > > > > > > > > 4. After you updated the FLIP, there are
> some
> > > > > > > > inconsistent
> > > > > > > > > > > > > > descriptions
> > > > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > the content. Would you like to clean them
> up?
> > > > > > Thanks!
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Best regards,
> > > > > > > > > > > > > > > > > Jing
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Fri, Apr 7, 2023 at 9:24 AM Feng Jin <
> > > > > > > > > > jinfeng1...@gmail.com
> > > > > > > > > > > >
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > hi Shammon
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > Thank you for your response, and I
> completely
> > > > > agree
> > > > > > > > with
> > > > > > > > > > your
> > > > > > > > > > > > > point
> > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > > view.
> > > > > > > > > > > > > > > > > > Initially, I may have over complicated
> the
> > > > whole
> > > > > > > issue.
> > > > > > > > > > First
> > > > > > > > > > > > and
> > > > > > > > > > > > > > > > > foremost,
> > > > > > > > > > > > > > > > > > we need to consider the persistence of
> the
> > > > > > Catalog's
> > > > > > > > > > > > > Configuration.
> > > > > > > > > > > > > > > > > > If we only need to provide persistence
> for
> > > > > Catalog
> > > > > > > > > > > > Configuration,
> > > > > > > > > > > > > > we
> > > > > > > > > > > > > > > > can
> > > > > > > > > > > > > > > > > > add a toConfiguration method to the
> Catalog
> > > > > > > interface.
> > > > > > > > > > > > > > > > > > This method can convert a Catalog
> instance
> > > to a
> > > > > > > > > Map<String,
> > > > > > > > > > > > > String>
> > > > > > > > > > > > > > > > > > properties, and the default
> implementation
> > > will
> > > > > > throw
> > > > > > > > an
> > > > > > > > > > > > > exception.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > public interface Catalog {
> > > > > > > > > > > > > > > > > > /**
> > > > > > > > > > > > > > > > > > * Returns a map containing the
> properties
> > > of
> > > > > the
> > > > > > > > > catalog
> > > > > > > > > > > > > object.
> > > > > > > > > > > > > > > > > > *
> > > > > > > > > > > > > > > > > > * @return a map containing the
> properties
> > > of
> > > > > the
> > > > > > > > > catalog
> > > > > > > > > > > > > object
> > > > > > > > > > > > > > > > > > * @throws
> UnsupportedOperationException if
> > > > the
> > > > > > > > > > > implementing
> > > > > > > > > > > > > > class
> > > > > > > > > > > > > > > > does
> > > > > > > > > > > > > > > > > > not override
> > > > > > > > > > > > > > > > > > * the default implementation of this
> > > method
> > > > > > > > > > > > > > > > > > */
> > > > > > > > > > > > > > > > > > default Map<String, String>
> toProperties()
> > > {
> > > > > > > > > > > > > > > > > > throw new
> > > > > UnsupportedOperationException("Please
> > > > > > > > > > implement
> > > > > > > > > > > > > > > > > toProperties
> > > > > > > > > > > > > > > > > > for this catalog");
> > > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > The specific process is as follows:
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > 1. If the user has configured a
> > > CatalogStore,
> > > > > the
> > > > > > > > > > > > > > toConfiguration()
> > > > > > > > > > > > > > > > > method
> > > > > > > > > > > > > > > > > > will be called when registering a Catalog
> > > > > instance
> > > > > > > with
> > > > > > > > > > > > > > > > > > registerCatalog(String catalogName,
> Catalog
> > > > > > catalog).
> > > > > > > > The
> > > > > > > > > > > > Catalog
> > > > > > > > > > > > > > > > > instance
> > > > > > > > > > > > > > > > > > will be converted to a Map<String,
> String>
> > > > > > properties
> > > > > > > > and
> > > > > > > > > > > saved
> > > > > > > > > > > > > to
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > CatalogStore. If some Catalog instances
> do
> > > not
> > > > > > > > implement
> > > > > > > > > > this
> > > > > > > > > > > > > > method,
> > > > > > > > > > > > > > > > an
> > > > > > > > > > > > > > > > > > exception will be thrown.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > 2. If the user does not use a
> CatalogStore,
> > > > the
> > > > > > > > > > > > > toConfiguration()
> > > > > > > > > > > > > > > > method
> > > > > > > > > > > > > > > > > > will not be called, ensuring consistency
> with
> > > > the
> > > > > > > > > original
> > > > > > > > > > > > > process.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > 3. Saving both the Map<String, Catalog>
> > > > catalogs
> > > > > > and
> > > > > > > > the
> > > > > > > > > > > > > > CatalogStore
> > > > > > > > > > > > > > > > at
> > > > > > > > > > > > > > > > > > the same time can also avoid conflicts
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > For lazy initialization:
> > > > > > > > > > > > > > > > > > we can start from the Catalog itself and
> > > > provide
> > > > > a
> > > > > > > > > > dedicated
> > > > > > > > > > > > > > Catalog
> > > > > > > > > > > > > > > > > > implementation for lazy loading, such as
> > > > > > > > > > ConfigurationCatalog
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > public class ConfigurationCatalog
> implements
> > > > > > Catalog
> > > > > > > {
> > > > > > > > > > > > > > > > > > ConfigurationCatalog(Map<String, String>
> > > > > > properties)
> > > > > > > {
> > > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > > > }
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > I have added this design to the FLIP.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > Best,
> > > > > > > > > > > > > > > > > > Feng
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > On Thu, Apr 6, 2023 at 10:03 AM Shammon
> FY <
> > > > > > > > > > > zjur...@gmail.com>
> > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > Hi Feng
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > Thanks for your answer.
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > I have no questions about the
> functionality
> > > > of
> > > > > > > > > > > > `CatalogStore`,
> > > > > > > > > > > > > > but
> > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > different operations of multiple
> > > > > > `registerCatalog`
> > > > > > > or
> > > > > > > > > > > > > > > `storeCatalog`
> > > > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > > > `CatalogManager`. The implementation in
> > > Trino
> > > > > is
> > > > > > > also
> > > > > > > > > the
> > > > > > > > > > > > same,
> > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > `CatalogManager` in trino only has one
> > > > > > > > `createCatalog`,
> > > > > > > > > > > which
> > > > > > > > > > > > > > will
> > > > > > > > > > > > > > > > save
> > > > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > catalog to memory and `CatalogStore`.
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > I think we don't need to add another
> > > > > > > > `registerCatalog`
> > > > > > > > > or
> > > > > > > > > > > > > > > > `addCatalog`
> > > > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > > > save a catalog in `Map<String, Catalog>
> > > > > catalogs`
> > > > > > > or
> > > > > > > > > > > > > > `CatalogStore
> > > > > > > > > > > > > > > > > > > catalogStore`. As you mentioned
> before,
> > > the
> > > > > > > > > `Map<String,
> > > > > > > > > > > > > > Catalog>
> > > > > > > > > > > > > > > > > > > catalogs` is a cache in
> `CatalogManager`.
> > > How
> > > > > > about
> > > > > > > > > > saving
> > > > > > > > > > > > the
> > > > > > > > > > > > > > > > catalog
> > > > > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > > > `Map<String, Catalog> catalogs` and
> > > > > `CatalogStore
> > > > > > > > > > > > catalogStore`
> > > > > > > > > > > > > > > > > together
> > > > > > > > > > > > > > > > > > > when it is registered or added in
> > > > > > `CatalogManager`?
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > Best,
> > > > > > > > > > > > > > > > > > > Shammon FY
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > On Tue, Apr 4, 2023 at 5:22 PM Feng
> Jin <
> > > > > > > > > > > > jinfeng1...@gmail.com
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > Thank you for your reply. I am very
> sorry
> > > > for
> > > > > > the
> > > > > > > > > > > > > > > misunderstanding
> > > > > > > > > > > > > > > > > > caused
> > > > > > > > > > > > > > > > > > > > by my deviation from the original
> > > > discussion.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > @Shammon
> > > > > > > > > > > > > > > > > > > > > I found there is a pre-discussion
> [1]
> > > for
> > > > > > this
> > > > > > > > FLIP
> > > > > > > > > > > > > > > > > > > > Yes, there was indeed such a
> discussion
> > > > > before.
> > > > > > > > > > However,
> > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > designing
> > > > > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > whole solution, I found that the
> logic of
> > > > > > > > > > CatalogManager
> > > > > > > > > > > > > itself
> > > > > > > > > > > > > > > > > doesn't
> > > > > > > > > > > > > > > > > > > > need to change much. *We cannot only
> > > > persist
> > > > > > > > Catalog
> > > > > > > > > > > > > instances
> > > > > > > > > > > > > > > > > > > themselves*,
> > > > > > > > > > > > > > > > > > > > so exposing only
> registerCatalog(String
> > > > > > > > catalogName,
> > > > > > > > > > > > Catalog
> > > > > > > > > > > > > > > > catalog)
> > > > > > > > > > > > > > > > > > > might
> > > > > > > > > > > > > > > > > > > > not be enough to save Catalogs,
> because
> > > in
> > > > > the
> > > > > > > end
> > > > > > > > we
> > > > > > > > > > > still
> > > > > > > > > > > > > > need
> > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > > save
> > > > > > > > > > > > > > > > > > > > the configurations corresponding to
> the
> > > > > Catalog
> > > > > > > > > > > instances.
> > > > > > > > > > > > > > > > > Therefore,
> > > > > > > > > > > > > > > > > > I
> > > > > > > > > > > > > > > > > > > > decided to introduce the CatalogStore
> > > > > interface
> > > > > > > for
> > > > > > > > > > > > > > configuration
> > > > > > > > > > > > > > > > > > > > persistence. Regarding this part, I
> also
> > > > took
> > > > > > > > > > inspiration
> > > > > > > > > > > > > from
> > > > > > > > > > > > > > > > > Trino's
> > > > > > > > > > > > > > > > > > > > implementation[1].
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > @yuxia
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > 1: The mechanism of handling
> catalog
> > > with
> > > > > the
> > > > > > > > same
> > > > > > > > > > name
> > > > > > > > > > > > > > looks a
> > > > > > > > > > > > > > > > > > little
> > > > > > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > > > > complex to me.
> > > > > > > > > > > > > > > > > > > > Thank you for the suggestion. I will
> > > > provide
> > > > > a
> > > > > > > > > detailed
> > > > > > > > > > > > > > > description
> > > > > > > > > > > > > > > > > and
> > > > > > > > > > > > > > > > > > > > code examples for this part, and add
> it
> > > to
> > > > > the
> > > > > > > > FLIP.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > 2: TBH, the method name
> `addCatalog`
> > > > still
> > > > > > > looks
> > > > > > > > > > > confused
> > > > > > > > > > > > > to
> > > > > > > > > > > > > > > me.
> > > > > > > > > > > > > > > > > > IIUC,
> > > > > > > > > > > > > > > > > > > > this method is for storing catalog to
> > > > > > > CatalogStore,
> > > > > > > > > how
> > > > > > > > > > > > about
> > > > > > > > > > > > > > > > > renaming
> > > > > > > > > > > > > > > > > > it
> > > > > > > > > > > > > > > > > > > > to `storeCatalog`? It's very personal
> > > > > opinion,
> > > > > > > you
> > > > > > > > > can
> > > > > > > > > > > > decide
> > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > take
> > > > > > > > > > > > > > > > > > it
> > > > > > > > > > > > > > > > > > > or
> > > > > > > > > > > > > > > > > > > > not by your self.
> > > > > > > > > > > > > > > > > > > > StoreCatalog looks more intuitive to
> me.
> > > I
> > > > > > don't
> > > > > > > > see
> > > > > > > > > > any
> > > > > > > > > > > > > > problem
> > > > > > > > > > > > > > > > with
> > > > > > > > > > > > > > > > > > it.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > 3.For CREATE CATALOG statement,
> which
> > > > > method
> > > > > > > will
> > > > > > > > > be
> > > > > > > > > > > > > called?
> > > > > > > > > > > > > > > > > > > > `registerCatalog` or `addCatalog`?
> I'm
> > > > > > wondering
> > > > > > > > > > whether
> > > > > > > > > > > > user
> > > > > > > > > > > > > > can
> > > > > > > > > > > > > > > > > add a
> > > > > > > > > > > > > > > > > > > > catalog to store with SQL stement.
> > > > > > > > > > > > > > > > > > > > For CREATE CATALOG, my original
> design
> > > was
> > > > to
> > > > > > add
> > > > > > > > it
> > > > > > > > > > > > directly
> > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > CatalogStore, but this would disrupt
> the
> > > > > > existing
> > > > > > > > > > logic.
> > > > > > > > > > > > > > > > Therefore, I
> > > > > > > > > > > > > > > > > > > think
> > > > > > > > > > > > > > > > > > > > we can do both: save the
> configuration to
> > > > the
> > > > > > > > > > > CatalogStore
> > > > > > > > > > > > > and
> > > > > > > > > > > > > > > > > > initialize
> > > > > > > > > > > > > > > > > > > > the Catalog instance at the same time
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > 3. Is it really neccessary to
> provide a
> > > > > > default
> > > > > > > > > > > > > implmentation
> > > > > > > > > > > > > > > for
> > > > > > > > > > > > > > > > > > > > interface `CatalogStoreFactory`?
> > > > > > > > > > > > > > > > > > > > I think it is necessary, otherwise we
> > > would
> > > > > > need
> > > > > > > to
> > > > > > > > > > > > introduce
> > > > > > > > > > > > > > an
> > > > > > > > > > > > > > > > > > > additional
> > > > > > > > > > > > > > > > > > > > Map to store the configuration for
> lazy
> > > > > > loading.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > 4: About asynchronous registration
> for
> > > > > > catalog.
> > > > > > > > > > > > > > > > > > > > I don't think registerCatalog(String
> > > > > > catalogName,
> > > > > > > > > > Catalog
> > > > > > > > > > > > > > > catalog)
> > > > > > > > > > > > > > > > > can
> > > > > > > > > > > > > > > > > > be
> > > > > > > > > > > > > > > > > > > > made into an asynchronous interface
> > > because
> > > > > > > Catalog
> > > > > > > > > is
> > > > > > > > > > > > > already
> > > > > > > > > > > > > > an
> > > > > > > > > > > > > > > > > > > instance.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > It looks more like lazy
> initialization
> > > > for
> > > > > > > > catalog
> > > > > > > > > > than
> > > > > > > > > > > > > > > > > asynchronous
> > > > > > > > > > > > > > > > > > > > registration, right?
> > > > > > > > > > > > > > > > > > > > Indeed, my description was
> inaccurate. It
> > > > > > should
> > > > > > > be
> > > > > > > > > > lazy
> > > > > > > > > > > > > > > > registration
> > > > > > > > > > > > > > > > > > > > instead of asynchronous
> registration. I
> > > > have
> > > > > > > > already
> > > > > > > > > > > > updated
> > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > title
> > > > > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > > > > the FLIP.
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > [1].
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> https://github.com/trinodb/trino/blob/master/core/trino-main/src/main/java/io/trino/connector/CatalogStore.java
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > On Tue, Apr 4, 2023 at 4:27 PM
> Shammon
> > > FY <
> > > > > > > > > > > > zjur...@gmail.com
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > Hi Feng
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > I think if there is a
> `registerCatalog`
> > > > > > method
> > > > > > > in
> > > > > > > > > > > > > > > > `CatalogManager`,
> > > > > > > > > > > > > > > > > > it
> > > > > > > > > > > > > > > > > > > > will
> > > > > > > > > > > > > > > > > > > > > confuse users whether a method
> named
> > > > > > > `addCatalog`
> > > > > > > > > or
> > > > > > > > > > > > > > > > `storeCatalog`
> > > > > > > > > > > > > > > > > > is
> > > > > > > > > > > > > > > > > > > > > added.
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > And as you mentioned, the memory
> > > catalog
> > > > > is a
> > > > > > > > > > `cache`,
> > > > > > > > > > > I
> > > > > > > > > > > > > > think
> > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > concept
> > > > > > > > > > > > > > > > > > > > > of `cache` should not be exposed to
> > > > users.
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > I found there is a pre-discussion
> [1]
> > > for
> > > > > > this
> > > > > > > > > FLIP.
> > > > > > > > > > > > Please
> > > > > > > > > > > > > > > > correct
> > > > > > > > > > > > > > > > > > me
> > > > > > > > > > > > > > > > > > > if
> > > > > > > > > > > > > > > > > > > > > I'm wrong, IIUC, the conclusion of
> that
> > > > > > > > discussion
> > > > > > > > > is
> > > > > > > > > > > to
> > > > > > > > > > > > > use
> > > > > > > > > > > > > > > > > > > > > `CatalogManager` as an interface
> and
> > > > > > implement
> > > > > > > it
> > > > > > > > > for
> > > > > > > > > > > > > > different
> > > > > > > > > > > > > > > > > > stores
> > > > > > > > > > > > > > > > > > > > such
> > > > > > > > > > > > > > > > > > > > > as memory, file and external
> system.
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > I think there is a gap between the
> > > > current
> > > > > > FLIP
> > > > > > > > > > design
> > > > > > > > > > > > and
> > > > > > > > > > > > > > that
> > > > > > > > > > > > > > > > > > > > conclusion.
> > > > > > > > > > > > > > > > > > > > > What about the proposal of the
> > > discussion
> > > > > in
> > > > > > > > thread
> > > > > > > > > > > [1] ?
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > [1]
> > > > > > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > https://lists.apache.org/thread/9bnjblgd9wvrl75lkm84oo654c4lqv70
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > Best,
> > > > > > > > > > > > > > > > > > > > > Shammon FY
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > On Tue, Apr 4, 2023 at 3:41 PM
> yuxia <
> > > > > > > > > > > > > > > > luoyu...@alumni.sjtu.edu.cn>
> > > > > > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > Thanks Feng for driving this
> FLIP.
> > > > > > > > > > > > > > > > > > > > > > I have few comments:
> > > > > > > > > > > > > > > > > > > > > > 1: The mechanism of handling
> catalog
> > > > with
> > > > > > the
> > > > > > > > > same
> > > > > > > > > > > name
> > > > > > > > > > > > > > > looks a
> > > > > > > > > > > > > > > > > > > little
> > > > > > > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > > > > > > complex to me. I think it'll be
> > > better
> > > > to
> > > > > > > > explain
> > > > > > > > > > it
> > > > > > > > > > > in
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > java
> > > > > > > > > > > > > > > > > > doc
> > > > > > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > > > > > > these methods and give a brief
> > > example
> > > > in
> > > > > > > this
> > > > > > > > > > FLIP.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 2: TBH, the method name
> `addCatalog`
> > > > > still
> > > > > > > > looks
> > > > > > > > > > > > confused
> > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > me.
> > > > > > > > > > > > > > > > > > > IIUC,
> > > > > > > > > > > > > > > > > > > > > > this method is for storing
> catalog to
> > > > > > > > > CatalogStore,
> > > > > > > > > > > how
> > > > > > > > > > > > > > about
> > > > > > > > > > > > > > > > > > > renaming
> > > > > > > > > > > > > > > > > > > > it
> > > > > > > > > > > > > > > > > > > > > > to `storeCatalog`? It's very
> personal
> > > > > > > opinion,
> > > > > > > > > you
> > > > > > > > > > > can
> > > > > > > > > > > > > > decide
> > > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > > take
> > > > > > > > > > > > > > > > > > > > it
> > > > > > > > > > > > > > > > > > > > > or
> > > > > > > > > > > > > > > > > > > > > > not by your self.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 3: For CREATE CATALOG statement,
> > > which
> > > > > > method
> > > > > > > > > will
> > > > > > > > > > be
> > > > > > > > > > > > > > called?
> > > > > > > > > > > > > > > > > > > > > > `registerCatalog` or
> `addCatalog`?
> > > I'm
> > > > > > > > wondering
> > > > > > > > > > > > whether
> > > > > > > > > > > > > > user
> > > > > > > > > > > > > > > > can
> > > > > > > > > > > > > > > > > > > add a
> > > > > > > > > > > > > > > > > > > > > > catalog to store with SQL
> stement.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 3: Is it really neccessary to
> > > provide a
> > > > > > > default
> > > > > > > > > > > > > > implmentation
> > > > > > > > > > > > > > > > for
> > > > > > > > > > > > > > > > > > > > > > interface `CatalogStoreFactory`?
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 4: About asynchronous
> registration
> > > for
> > > > > > > catalog.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > When creating a catalog with
> CREATE
> > > > > > > CATALOG,
> > > > > > > > > the
> > > > > > > > > > > > > > > asynchronous
> > > > > > > > > > > > > > > > > > > > > > registration method is used by
> > > default.
> > > > > > > > > > > > > > > > > > > > > > If asynchronous registration is
> the
> > > > > default
> > > > > > > > > > behavior,
> > > > > > > > > > > > it
> > > > > > > > > > > > > > > there
> > > > > > > > > > > > > > > > > any
> > > > > > > > > > > > > > > > > > > way
> > > > > > > > > > > > > > > > > > > > > > that user can switch to
> synchronous
> > > > > > > > registration
> > > > > > > > > > just
> > > > > > > > > > > > > like
> > > > > > > > > > > > > > > > > before?
> > > > > > > > > > > > > > > > > > > > > > Will both method `addCatalog` and
> > > > > > > > > `registerCatalog`
> > > > > > > > > > > be
> > > > > > > > > > > > > > > > > asynchronous
> > > > > > > > > > > > > > > > > > > > > > registration?
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > IIUC, in asynchronous
> registration,
> > > it
> > > > > may
> > > > > > > well
> > > > > > > > > > that
> > > > > > > > > > > > > CREATE
> > > > > > > > > > > > > > > > > CATALOG
> > > > > > > > > > > > > > > > > > > > > > executes successfully, but then
> the
> > > > > > following
> > > > > > > > > > CREATE
> > > > > > > > > > > > > TABLE
> > > > > > > > > > > > > > > > > > statement
> > > > > > > > > > > > > > > > > > > > will
> > > > > > > > > > > > > > > > > > > > > > fail for the catalog fail to
> open.
> > > > > > > > > > > > > > > > > > > > > > I think it's a break change which
> > > > should
> > > > > be
> > > > > > > > > > > highlighted
> > > > > > > > > > > > > in
> > > > > > > > > > > > > > > this
> > > > > > > > > > > > > > > > > > FLIP,
> > > > > > > > > > > > > > > > > > > > may
> > > > > > > > > > > > > > > > > > > > > > be in compatibility part.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > BTW, by saying asynchronous
> > > > > registration, I
> > > > > > > > would
> > > > > > > > > > > like
> > > > > > > > > > > > to
> > > > > > > > > > > > > > > > expect
> > > > > > > > > > > > > > > > > > > there
> > > > > > > > > > > > > > > > > > > > > > will be an executor to open or
> > > register
> > > > > > > catalog
> > > > > > > > > in
> > > > > > > > > > > the
> > > > > > > > > > > > > > > > > background,
> > > > > > > > > > > > > > > > > > > but
> > > > > > > > > > > > > > > > > > > > > from
> > > > > > > > > > > > > > > > > > > > > > your previous comments,
> > > > > > > > > > > > > > > > > > > > > > "the Catalog instance will be
> > > > initialized
> > > > > > if
> > > > > > > it
> > > > > > > > > has
> > > > > > > > > > > not
> > > > > > > > > > > > > > been
> > > > > > > > > > > > > > > > > > > > initialized
> > > > > > > > > > > > > > > > > > > > > > yet. If the initialization
> process
> > > > fails,
> > > > > > > these
> > > > > > > > > > > > > statements
> > > > > > > > > > > > > > > will
> > > > > > > > > > > > > > > > > not
> > > > > > > > > > > > > > > > > > > be
> > > > > > > > > > > > > > > > > > > > > > executed successfully."
> > > > > > > > > > > > > > > > > > > > > > It looks more like lazy
> > > initialization
> > > > > for
> > > > > > > > > catalog
> > > > > > > > > > > than
> > > > > > > > > > > > > > > > > > asynchronous
> > > > > > > > > > > > > > > > > > > > > > registration, right?
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > Best regards,
> > > > > > > > > > > > > > > > > > > > > > Yuxia
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > ----- 原始邮件 -----
> > > > > > > > > > > > > > > > > > > > > > 发件人: "Feng Jin" <
> > > jinfeng1...@gmail.com
> > > > >
> > > > > > > > > > > > > > > > > > > > > > 收件人: "dev" <dev@flink.apache.org
> >
> > > > > > > > > > > > > > > > > > > > > > 发送时间: 星期一, 2023年 4 月 03日 下午
> 3:27:45
> > > > > > > > > > > > > > > > > > > > > > 主题: Re: [DISCUSS] FLIP 295:
> Support
> > > > > > > persistence
> > > > > > > > > of
> > > > > > > > > > > > > Catalog
> > > > > > > > > > > > > > > > > > > > configuration
> > > > > > > > > > > > > > > > > > > > > > and asynchronous registration
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > Hi everyone, Thank you all for
> your
> > > > > > interest
> > > > > > > in
> > > > > > > > > > this
> > > > > > > > > > > > > > DISCUSS.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > @Shammon
> > > > > > > > > > > > > > > > > > > > > > > How to handle a catalog with
> the
> > > same
> > > > > > name
> > > > > > > > that
> > > > > > > > > > > > exists
> > > > > > > > > > > > > > for
> > > > > > > > > > > > > > > > both
> > > > > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > > > > > them?
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > I believe this is a crucial
> point.
> > > > Based
> > > > > on
> > > > > > > my
> > > > > > > > > > > current
> > > > > > > > > > > > > > > personal
> > > > > > > > > > > > > > > > > > > > > > understanding, the Map<String,
> > > Catalog>
> > > > > > > > catalogs
> > > > > > > > > > will
> > > > > > > > > > > > > serve
> > > > > > > > > > > > > > > as
> > > > > > > > > > > > > > > > a
> > > > > > > > > > > > > > > > > > > cache
> > > > > > > > > > > > > > > > > > > > > > for instantiated catalogs and
> have
> > > the
> > > > > > > highest
> > > > > > > > > > > > priority.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > There are three methods that can
> > > affect
> > > > > the
> > > > > > > > > > > Map<String,
> > > > > > > > > > > > > > > > Catalog>
> > > > > > > > > > > > > > > > > > > > > catalogs:
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 1. registerCatalog(String
> > > catalogName,
> > > > > > > Catalog
> > > > > > > > > > > catalog)
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > This method puts the catalog
> instance
> > > > > into
> > > > > > > the
> > > > > > > > > > > > > Map<String,
> > > > > > > > > > > > > > > > > Catalog>
> > > > > > > > > > > > > > > > > > > > > > catalogs.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 2. unregisterCatalog(String
> > > > catalogName)
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > This method removes the catalog
> > > > instance
> > > > > > > > > > > corresponding
> > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > > catalogName
> > > > > > > > > > > > > > > > > > > > > > from the Map<String, Catalog>
> > > catalogs.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 3. getCatalog(String catalogName)
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > This method first retrieves the
> > > > > > corresponding
> > > > > > > > > > catalog
> > > > > > > > > > > > > > > instance
> > > > > > > > > > > > > > > > > from
> > > > > > > > > > > > > > > > > > > > > > the Map<String, Catalog>
> catalogs. If
> > > > the
> > > > > > > > catalog
> > > > > > > > > > > does
> > > > > > > > > > > > > not
> > > > > > > > > > > > > > > > exist,
> > > > > > > > > > > > > > > > > > it
> > > > > > > > > > > > > > > > > > > > > > retrieves the corresponding
> > > > configuration
> > > > > > > from
> > > > > > > > > the
> > > > > > > > > > > > > > > > CatalogStore,
> > > > > > > > > > > > > > > > > > > > > > initializes it, and puts the
> > > > initialized
> > > > > > > > Catalog
> > > > > > > > > > > > instance
> > > > > > > > > > > > > > > into
> > > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > Map<String, Catalog> catalogs.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > The following two methods only
> modify
> > > > the
> > > > > > > > > > > configuration
> > > > > > > > > > > > > in
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > CatalogStore:
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 1. addCatalog(String catalogName,
> > > > > > Map<String,
> > > > > > > > > > String>
> > > > > > > > > > > > > > > > properties)
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > This method saves the properties
> to
> > > the
> > > > > > > > > > catalogStore
> > > > > > > > > > > > and
> > > > > > > > > > > > > > > checks
> > > > > > > > > > > > > > > > > > > > > > whether there is a catalogName
> with
> > > the
> > > > > > same
> > > > > > > > > name.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 2. removeCatalog(String
> catalogName)
> > > > > > > > > > > > > > > > > > > > > > This method removes the specified
> > > > > > > configuration
> > > > > > > > > of
> > > > > > > > > > > the
> > > > > > > > > > > > > > > > specified
> > > > > > > > > > > > > > > > > > > > > > catalogName in the catalogStore.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > The following are possible
> conflict
> > > > > > > scenarios:
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 1. When the corresponding
> catalogName
> > > > > > already
> > > > > > > > > > exists
> > > > > > > > > > > in
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > CatalogStore but not in the
> > > Map<String,
> > > > > > > > Catalog>,
> > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > registerCatalog(String
> catalogName,
> > > > > Catalog
> > > > > > > > > > catalog)
> > > > > > > > > > > > > method
> > > > > > > > > > > > > > > can
> > > > > > > > > > > > > > > > > > > > > > succeed and be directly saved to
> the
> > > > > > > > Map<String,
> > > > > > > > > > > > Catalog>
> > > > > > > > > > > > > > > > > catalogs.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 2. When the corresponding
> catalogName
> > > > > > already
> > > > > > > > > > exists
> > > > > > > > > > > in
> > > > > > > > > > > > > > both
> > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > CatalogStore and the Map<String,
> > > > > Catalog>,
> > > > > > > the
> > > > > > > > > > > > > > > > > > registerCatalog(String
> > > > > > > > > > > > > > > > > > > > > > catalogName, Catalog catalog)
> method
> > > > will
> > > > > > > fail.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > 3. When the corresponding
> catalogName
> > > > > > already
> > > > > > > > > > exists
> > > > > > > > > > > in
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > Map<String, Catalog>, the
> > > > > addCatalog(String
> > > > > > > > > > > > catalogName,
> > > > > > > > > > > > > > > > > > Map<String,
> > > > > > > > > > > > > > > > > > > > > > String> properties) method can
> > > directly
> > > > > > save
> > > > > > > > the
> > > > > > > > > > > > > properties
> > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > catalogStore, but the
> > > getCatalog(String
> > > > > > > > > > catalogName)
> > > > > > > > > > > > > method
> > > > > > > > > > > > > > > > will
> > > > > > > > > > > > > > > > > > not
> > > > > > > > > > > > > > > > > > > > > > use the new properties for
> > > > initialization
> > > > > > > > because
> > > > > > > > > > the
> > > > > > > > > > > > > > > > > corresponding
> > > > > > > > > > > > > > > > > > > > > > catalog instance already exists
> in
> > > > > catalogs
> > > > > > > and
> > > > > > > > > > will
> > > > > > > > > > > be
> > > > > > > > > > > > > > > > > > prioritized.
> > > > > > > > > > > > > > > > > > > > > > Therefore, using the
> > > > > > unregisterCatalog(String
> > > > > > > > > > > > > catalogName)
> > > > > > > > > > > > > > > > method
> > > > > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > > > > > > remove the instance
> corresponding to
> > > > the
> > > > > > > > original
> > > > > > > > > > > > > > catalogName
> > > > > > > > > > > > > > > > is
> > > > > > > > > > > > > > > > > > > > > > necessary.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > I think it will confuse users
> that
> > > > > > > > > > > > > > `registerCatalog(String
> > > > > > > > > > > > > > > > > > > > > > catalogName,Catalog catalog)` in
> the
> > > > > > > > `Map<String,
> > > > > > > > > > > > > Catalog>
> > > > > > > > > > > > > > > > > > catalogs`
> > > > > > > > > > > > > > > > > > > > and
> > > > > > > > > > > > > > > > > > > > > > `registerCatalog(String
> catalogName,
> > > > > > > > Map<String,
> > > > > > > > > > > > String>
> > > > > > > > > > > > > > > > > > properties)
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > This could potentially lead to
> > > > > confusion. I
> > > > > > > > > suggest
> > > > > > > > > > > > > > changing
> > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > method name, perhaps to
> > > > addCatalog(String
> > > > > > > > > > > catalogName,
> > > > > > > > > > > > > > > > > Map<String,
> > > > > > > > > > > > > > > > > > > > > > String> properties), as
> previously
> > > > > > mentioned
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > @Hang
> > > > > > > > > > > > > > > > > > > > > > > add `registerCatalog(String
> > > > > > > > catalogName,Catalog
> > > > > > > > > > > > > catalog,
> > > > > > > > > > > > > > > > > > > > > > boolean lazyInit)` method
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > Since a catalog is already an
> > > instance,
> > > > > > > adding
> > > > > > > > > the
> > > > > > > > > > > > > > "lazyInit"
> > > > > > > > > > > > > > > > > > > > > > parameter to the
> > > registerCatalog(String
> > > > > > > > > > catalogName,
> > > > > > > > > > > > > > Catalog
> > > > > > > > > > > > > > > > > > catalog)
> > > > > > > > > > > > > > > > > > > > > > method may not necessarily
> result in
> > > > lazy
> > > > > > > > > > > > initialization.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > Do we need to think about
> > > encryption
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > I think encryption is necessary,
> but
> > > > > > perhaps
> > > > > > > > the
> > > > > > > > > > > > > encryption
> > > > > > > > > > > > > > > > logic
> > > > > > > > > > > > > > > > > > > > > > should be implemented in the
> > > > CatalogStore
> > > > > > > when
> > > > > > > > > the
> > > > > > > > > > > data
> > > > > > > > > > > > > is
> > > > > > > > > > > > > > > > > actually
> > > > > > > > > > > > > > > > > > > > > > saved. For instance, the
> > > > FileCatalogStore
> > > > > > > could
> > > > > > > > > > > encrypt
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > data
> > > > > > > > > > > > > > > > > > when
> > > > > > > > > > > > > > > > > > > > > > it is saved to a file, while the
> > > > > > > > > MemoryCatalogStore
> > > > > > > > > > > > does
> > > > > > > > > > > > > > not
> > > > > > > > > > > > > > > > > > require
> > > > > > > > > > > > > > > > > > > > > > encryption.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > Do we really need the
> > > > > > > `MemoryCatalogStore`?
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > I think it is necessary to have a
> > > > > > > > > > MemoryCatalogStore
> > > > > > > > > > > as
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > default
> > > > > > > > > > > > > > > > > > > > > > implementation, which saves
> Catalog
> > > > > > > > > configurations
> > > > > > > > > > in
> > > > > > > > > > > > > > memory.
> > > > > > > > > > > > > > > > > > > > > > Otherwise, if we want to
> implement
> > > > > > > asynchronous
> > > > > > > > > > > loading
> > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > Catalog,
> > > > > > > > > > > > > > > > > > > we
> > > > > > > > > > > > > > > > > > > > > > would need to introduce
> additional
> > > > cache.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > @Xianxun
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > What if asynchronous
> registration
> > > > > failed?
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > This is also a critical concern.
> > > When
> > > > > > > > executing
> > > > > > > > > > DDL,
> > > > > > > > > > > > > DQL,
> > > > > > > > > > > > > > or
> > > > > > > > > > > > > > > > DML
> > > > > > > > > > > > > > > > > > > > > > statements that reference a
> specific
> > > > > > Catalog,
> > > > > > > > the
> > > > > > > > > > > > Catalog
> > > > > > > > > > > > > > > > > instance
> > > > > > > > > > > > > > > > > > > > > > will be initialized if it has not
> > > been
> > > > > > > > > initialized
> > > > > > > > > > > yet.
> > > > > > > > > > > > > If
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > initialization process fails,
> these
> > > > > > > statements
> > > > > > > > > will
> > > > > > > > > > > not
> > > > > > > > > > > > > be
> > > > > > > > > > > > > > > > > executed
> > > > > > > > > > > > > > > > > > > > > > successfully.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > nt. The Map<String, Catalog>
> > > catalogs
> > > > > and
> > > > > > > > > > > > CatalogStore
> > > > > > > > > > > > > > > > > > catalogstore
> > > > > > > > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > > > > > > the CatalogManager all have the
> same
> > > > > > catalog
> > > > > > > > > name,
> > > > > > > > > > > but
> > > > > > > > > > > > > > > > > correponding
> > > > > > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > > > > > > different catalog instances.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > This issue can be resolved by
> > > referring
> > > > > to
> > > > > > > the
> > > > > > > > > > > previous
> > > > > > > > > > > > > > > > > responses.
> > > > > > > > > > > > > > > > > > > The
> > > > > > > > > > > > > > > > > > > > > > key principle is that the Catalog
> > > that
> > > > > has
> > > > > > > been
> > > > > > > > > > most
> > > > > > > > > > > > > > recently
> > > > > > > > > > > > > > > > > used
> > > > > > > > > > > > > > > > > > > > > > will be given priority for
> subsequent
> > > > > use.
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > On Sun, Apr 2, 2023 at 10:58 PM
> > > Xianxun
> > > > > Ye
> > > > > > <
> > > > > > > > > > > > > > > > > > yesorno828...@gmail.com>
> > > > > > > > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > Hi Feng,
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > Thanks for driving this Flip,
> I do
> > > > > > believe
> > > > > > > > this
> > > > > > > > > > > Flip
> > > > > > > > > > > > > > could
> > > > > > > > > > > > > > > be
> > > > > > > > > > > > > > > > > > > helpful
> > > > > > > > > > > > > > > > > > > > > > for users. The firm I work for
> also
> > > > > > manages a
> > > > > > > > lot
> > > > > > > > > > of
> > > > > > > > > > > > > > > catalogs,
> > > > > > > > > > > > > > > > > and
> > > > > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > submission of tasks becomes slow
> > > > because
> > > > > of
> > > > > > > > > > loading a
> > > > > > > > > > > > > > number
> > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > > > > catalogs.
> > > > > > > > > > > > > > > > > > > > > > We obtain the catalogs in the
> user's
> > > > > Flink
> > > > > > > SQL
> > > > > > > > > > > through
> > > > > > > > > > > > > > > regular
> > > > > > > > > > > > > > > > > > > > expression
> > > > > > > > > > > > > > > > > > > > > > to avoid loading all catalogs to
> > > > improve
> > > > > > the
> > > > > > > > > speed
> > > > > > > > > > of
> > > > > > > > > > > > > task
> > > > > > > > > > > > > > > > > > > submission.
> > > > > > > > > > > > > > > > > > > > > > After reading flip-295, I have
> some
> > > > > > > questions:
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > 1. When creating a catalog with
> > > > CREATE
> > > > > > > > CATALOG,
> > > > > > > > > > the
> > > > > > > > > > > > > > > > > asynchronous
> > > > > > > > > > > > > > > > > > > > > > registration method is used by
> > > default.
> > > > > > What
> > > > > > > if
> > > > > > > > > > > > > > asynchronous
> > > > > > > > > > > > > > > > > > > > registration
> > > > > > > > > > > > > > > > > > > > > > failed? And how to implement
> > > > asynchronous
> > > > > > > > > > > registration?
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > 2. I also have the same
> question
> > > with
> > > > > > > > Shammon’s
> > > > > > > > > > > first
> > > > > > > > > > > > > > > > comment.
> > > > > > > > > > > > > > > > > > The
> > > > > > > > > > > > > > > > > > > > > > Map<String, Catalog> catalogs and
> > > > > > > CatalogStore
> > > > > > > > > > > > > catalogstore
> > > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > CatalogManager all have the same
> > > > catalog
> > > > > > > name,
> > > > > > > > > but
> > > > > > > > > > > > > > > correponding
> > > > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > > > > > > different catalog instances. For
> > > > example:
> > > > > > > > > > catalog1->
> > > > > > > > > > > > > > > > jdbcCatalog
> > > > > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > > > > > catalogs, but catalog1 ->
> hiveCatalog
> > > > in
> > > > > > the
> > > > > > > > > > > > > catalogstore.
> > > > > > > > > > > > > > > > Would
> > > > > > > > > > > > > > > > > > this
> > > > > > > > > > > > > > > > > > > > > case
> > > > > > > > > > > > > > > > > > > > > > happen? Maybe we should define
> > > > `catalogs`
> > > > > > and
> > > > > > > > > > > > > > `catalogstore`
> > > > > > > > > > > > > > > > more
> > > > > > > > > > > > > > > > > > > > > clearly.
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > Best regards,
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > Xianxun
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > > 2023年3月31日 11:13，Hang Ruan <
> > > > > > > > > > > ruanhang1...@gmail.com
> > > > > > > > > > > > >
> > > > > > > > > > > > > > 写道：
> > > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > > > MemoryCatalogStore
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
