Hi Jing Ge, First, flink-table-common contains all common classes of Flink Table, I think it is hard to bypass its dependence.
Secondly, almost all methods in Catalog looks useful to me, so if we are following LoD, we should add all methods again to TableEnvironment. I think it is redundant. And, this API chain does not look deep. - "tEnv.getCatalog(tEnv.getCurrentCatalog()).get().listDatabases()" looks a little complicated. The complex part is ahead. - If we have a method to get Catalog directly, can be simplify to "tEnv.catalog().listDatabase()", this is simple. Best, Jingsong On Thu, Feb 23, 2023 at 4:47 PM Jing Ge <j...@ververica.com.invalid> wrote: > > Hi Jingson, > > Thanks for the knowledge sharing. IMHO, it looks more like a design > guideline question than just avoiding public API change. Please correct me > if I'm wrong. > > Catalog is in flink-table-common module and TableEnvironment is in > flink-table-api-java. Depending on how and where those features proposed in > this FLIP will be used, we'd better reduce the dependency chain and follow > the Law of Demeter(LoD, clean code) [1]. Adding a new method in > TableEnvironment is therefore better than calling an API chain. It is also > more user friendly for the caller, because there is no need to understand > the internal structure of the called API. The downside of doing this is > that we might have another issue with the current TableEnvironment design - > the TableEnvironment interface got enlarged with more wrapper methods. This > is a different issue that could be solved with improved abstraction design > in the future. After considering pros and cons, if we want to add those > features now, I would prefer following LoD than API chain calls. WDYT? > > Best regards, > Jing > > [1] > https://hackernoon.com/object-oriented-tricks-2-law-of-demeter-4ecc9becad85 > > On Thu, Feb 23, 2023 at 6:26 AM Ran Tao <chucheng...@gmail.com> wrote: > > > Hi Jingsong. thanks. i got it. > > In this way, there is no need to introduce new API changes. > > > > Best Regards, > > Ran Tao > > > > > > Jingsong Li <jingsongl...@gmail.com> 于2023年2月23日周四 12:26写道： > > > > > Hi Ran, > > > > > > I mean we can just use > > > TableEnvironment.getCatalog(getCurrentCatalog).get().listDatabases(). > > > > > > We don't need to provide new apis just for utils. > > > > > > Best, > > > Jingsong > > > > > > On Thu, Feb 23, 2023 at 12:11 PM Ran Tao <chucheng...@gmail.com> wrote: > > > > > > > > Hi Jingsong, thanks. > > > > > > > > The implementation of these statements in TableEnvironmentImpl is > > called > > > > through the catalog api. > > > > > > > > but it does support some new override methods on the catalog api side, > > > and > > > > I will update it later. Thank you. > > > > > > > > e.g. > > > > TableEnvironmentImpl > > > > @Override > > > > public String[] listDatabases() { > > > > return catalogManager > > > > .getCatalog(catalogManager.getCurrentCatalog()) > > > > .get() > > > > .listDatabases() > > > > .toArray(new String[0]); > > > > } > > > > > > > > Best Regards, > > > > Ran Tao > > > > > > > > > > > > Jingsong Li <jingsongl...@gmail.com> 于2023年2月23日周四 11:47写道： > > > > > > > > > Thanks for the proposal. > > > > > > > > > > +1 for the proposal. > > > > > > > > > > I am confused about "Proposed TableEnvironment SQL API Changes", can > > > > > we just use catalog api for this requirement? > > > > > > > > > > Best, > > > > > Jingsong > > > > > > > > > > On Thu, Feb 23, 2023 at 10:48 AM Jacky Lau <liuyong...@gmail.com> > > > wrote: > > > > > > > > > > > > Hi Ran: > > > > > > Thanks for driving the FLIP. the google doc looks really good. it > > is > > > > > > important to improve user interactive experience. +1 to support > > this > > > > > > feature. > > > > > > > > > > > > Jing Ge <j...@ververica.com.invalid> 于2023年2月23日周四 00:51写道： > > > > > > > > > > > > > Hi Ran, > > > > > > > > > > > > > > Thanks for driving the FLIP. It looks overall good. Would you > > > like to > > > > > add > > > > > > > a description of useLike and notLike? I guess useLike true is for > > > > > "LIKE" > > > > > > > and notLike true is for "NOT LIKE" but I am not sure if I > > > understood it > > > > > > > correctly. Furthermore, does it make sense to support "ILIKE" > > too? > > > > > > > > > > > > > > Best regards, > > > > > > > Jing > > > > > > > > > > > > > > On Wed, Feb 22, 2023 at 1:17 PM Ran Tao <chucheng...@gmail.com> > > > wrote: > > > > > > > > > > > > > > > Currently flink sql auxiliary statements has supported some > > good > > > > > features > > > > > > > > such as catalog/databases/table support. > > > > > > > > > > > > > > > > But these features are not very complete compared with other > > > popular > > > > > > > > engines such as spark, presto, hive and commercial engines such > > > as > > > > > > > > snowflake. > > > > > > > > > > > > > > > > For example, many engines support show operation with filtering > > > > > except > > > > > > > > flink, and support describe other object(flink only support > > > describe > > > > > > > > table). > > > > > > > > > > > > > > > > I wonder can we add these useful features for flink? > > > > > > > > You can find details in this doc.[1] or FLIP.[2] > > > > > > > > > > > > > > > > Also, please let me know if there is a mistake. Looking forward > > > to > > > > > your > > > > > > > > reply. > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://docs.google.com/document/d/1hAiOfPx14VTBTOlpyxG7FA2mB1k5M31VnKYad2XpJ1I/ > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-297%3A+Improve+Auxiliary+Sql+Statements > > > > > > > > > > > > > > > > Best Regards, > > > > > > > > Ran Tao > > > > > > > > > > > > > > > > > > > > > > > > >
