Introducing a new term "path" to APIs like "getShortPath(Identifier)/getLongPath(Identifier)" would be confusing to users, thus I feel "getSimpleName/getIdentifier" is fine.
To summarize the discussion result. - categorize functions into 2 dimensions - system v.s. catalog, non-temp v.s. temp - and that give us 4 combinations - definition of FunctionIdentifier @PublicEvolving Class FunctionIdentifier { String name; ObjectIdentifier oi; // for temporary/non-temporary system function public FunctionIdentifier of(String name) { } // for temporary/non-temporary catalog function public FunctionIdentifier of(ObjectIdentifier identifier) { } Optional<ObjectIdentifier> getIdentifier() {} Optional<String> getSimpleName() {} } I've updated them to FLIP wiki. Please take a final look. I'll close the voting if there's no other concern raised within 24 hours. Cheers On Wed, Oct 2, 2019 at 4:54 AM Dawid Wysakowicz <dwysakow...@apache.org> wrote: > Hi, > > I very much agree with Xuefu's summary of the two points, especially on > the "functionIdentifier doesn't need to reflect the categories". > > For the factory methods I think methods of should be enough: > > // for temporary/non-temporary system function > public FunctionIdentifier of(String name) { } > // for temporary/non-temporary catalog function > public FunctionIdentifier of(ObjectIdentifier identifier){ } > > In case of the getters I did not like the method name `getName` in the > original proposal, as in my opinion it could imply that it can return > also just the name part of an ObjectIdentifier, which should not be the > case. > > I'm fine with getSimpleName/getIdentifier, but want to throw in few > other suggestions: > > * getShortPath(Identifier)/getLongPath(Identifier), > > * getSystemPath(Identifier)/getCatalogPath(Identifier) > > +1 to any of the 3 options. > > One additional thing the FunctionIdentifier should be a PublicEvolving > class, as it is part of a PublicEvolving APIs e.g. CallExpression, which > user might need to access e.g. in a filter pushdown. > > I also support the Xuefu's suggestion not to support the "ALL" keyword > in the "SHOW [TEMPORARY] FUNCTIONS" statement, but as the exact design > of it is not part of the FLIP-57, we do not need to agree on that in > this thread. > > Overall I think after updating the FLIP with the outcome of the > discussion I vote +1 for it. > > Best, > > Dawid > > > On 02/10/2019 00:21, Xuefu Z wrote: > > Here are some of my thoughts on the minor debates above: > > > > 1. +1 for 4 categories of functions. They are categorized along two > > dimensions of binary values: X: *temporary* vs non-temporary > (persistent); > > Y: *system* vs non-system (so said catalog). > > 2. In my opinion, class functionIdentifier doesn't really need to reflect > > the categories of the functions. Instead, we should decouple them to make > > the API more stable. Thus, my suggestion is: > > > > @Internal > > class FunctionIdentifier { > > // for temporary/non-temporary system function > > public FunctionIdentifier ofSimpleName(String name) { } > > // for temporary/non-temporary catalog function > > public FunctionIdentifier ofIdentifier(ObjectIdentifier > > identifier){ } > > public Optional<String> getSimpleName() { } > > public Optional<ObjectIdentifier> getIdentifier() { } > > } > > 3. DDLs -- I don't think we need "ALL" keyword. The grammar can just be: > > > > SHOW [TEMPORARY] [SYSTEM] FUNCTIONS. > > > > When either keyword is missing, "ALL" is implied along that dimension. We > > should always limit the search to the system function catalog and the > > current catalog/DB. I don't see a need of listing functions across > > different catalogs and databases. (It can be added later if that arises.) > > > > Thanks, > > Xuefu > > > > On Tue, Oct 1, 2019 at 11:12 AM Bowen Li <bowenl...@gmail.com> wrote: > > > >> Hi Dawid, > >> > >> Thanks for bringing the suggestions up. I was prototyping yesterday and > >> found out those places exactly as what you suggested. > >> > >> For CallExpression and UnresolvedCallExpression, I've added them to > >> FLIP-57. We will replace ObjectIdentifier with FunctionIdentifier and > mark > >> that as a breaking change > >> > >> For FunctionIdentifier, the suggested changes LGTM. Just want to bring > up > >> an issue on naming. It seems to me how we now name functions categories > is > >> a bit unclear and confusing, which is reflected on the suggested APIs - > in > >> FunctionIdentifier you lay out, "builtin function" would include builtin > >> functions and temporary system functions as we are kind of using > "system" > >> and "built-in" interchangeably, and "catalog function" would include > >> catalog functions and temporary functions. I currently can see two > >> approaches to make it clearer to users. > >> > >> 1) Simplify FunctionIdentifier to be the following. As it's internal, we > >> add comments and explanation to devs on which case the APIs support. > >> However, I feel this approach would be somehow a bit conflicting to what > >> you want to achieve for the clarity of APIs > >> > >> @Internal > >> class FunctionIdentifier { > >> // for built-in function and temporary system function > >> public FunctionIdentifier of(String name) { } > >> // for temporary function and catalog function > >> public FunctionIdentifier of(ObjectIdentifier identifier){ } > >> public Optional<String> getFunctionName() { } > >> public Optional<ObjectIdentifier> getObjectIdentifier() { } > >> } > >> > >> 2) We can rename our function categories as following so there'll be > mainly > >> just two categories of functions, "system functions" and "catalog > >> functions", either of which can have temporary ones > >> > >> - built-in functions -> officially rename to "system functions" and > note > >> to users that "system" and "built-in" can be used interchangeably. We > >> prefer "system" because that's the keyword we decided to use in DDL that > >> creates its temporary peers ("CREATE TEMPORARY SYSTEM FUNCTION") > >> - temporary system functions > >> - catalog functions > >> - temporary functions -> rename to "temporary catalog function" > >> > >> @Internal > >> class FunctionIdentifier { > >> // for temporary/non-temporary system function > >> public FunctionIdentifier ofSystemFunction(String name) { } > >> // for temporary/non-temporary catalog function > >> public FunctionIdentifier ofCatalogFunction(ObjectIdentifier > >> identifier){ } > >> public Optional<String> getSystemFunctionName() { } > >> public Optional<ObjectIdentifier> getCatalogFunctionIdentifier() { > } > >> } > >> > >> WDYT? > >> > >> > >> On Tue, Oct 1, 2019 at 5:48 AM Fabian Hueske <fhue...@gmail.com> wrote: > >> > >>> Thanks for the summary Bowen! > >>> > >>> Looks good to me. > >>> > >>> Cheers, > >>> Fabian > >>> > >>> Am Mo., 30. Sept. 2019 um 23:24 Uhr schrieb Bowen Li < > >> bowenl...@gmail.com > >>>> : > >>>> Hi all, > >>>> > >>>> I've updated the FLIP wiki with the following changes: > >>>> > >>>> - Lifespan of temp functions are not tied to those of catalogs and > >>>> databases. Users can create temp functions even though catalogs/dbs in > >>>> their fully qualified names don't even exist. > >>>> - some new SQL commands > >>>> - "SHOW FUNCTIONS" - list names of temp and non-temp > >> system/built-in > >>>> functions, and names of temp and catalog functions in the current > >> catalog > >>>> and db > >>>> - "SHOW ALL FUNCTIONS" - list names of temp and non-temp > >> system/built > >>>> functions, and fully qualified names of temp and catalog functions in > >> all > >>>> catalogs and dbs > >>>> - "SHOW ALL TEMPORARY FUNCTIONS" - list fully qualified names of > >> temp > >>>> functions in all catalog and db > >>>> - "SHOW ALL TEMPORARY SYSTEM FUNCTIONS" - list names of all temp > >>> system > >>>> functions > >>>> > >>>> Let me know if you have any questions. > >>>> > >>>> Seems we have resolved all concerns. If there's no more ones, I'd like > >> to > >>>> close the vote by this time tomorrow. > >>>> > >>>> Cheers, > >>>> Bowen > >>>> > >>>> On Mon, Sep 30, 2019 at 11:59 AM Bowen Li <bowenl...@gmail.com> > wrote: > >>>> > >>>>> Hi, > >>>>> > >>>>> I think above are some valid points, and we can adopt the > >> suggestions. > >>>>> To elaborate a bit on the new SQL syntax, it would imply that, unlike > >>>>> "SHOW FUNCTION" which only return function names, "SHOW ALL > >> [TEMPORARY] > >>>>> FUNCTIONS" would return functions' fully qualified names with catalog > >>> and > >>>>> db names. > >>>>> > >>>>> > >>>>> > >>>>> On Mon, Sep 30, 2019 at 6:38 AM Timo Walther <twal...@apache.org> > >>> wrote: > >>>>>> Hi all, > >>>>>> > >>>>>> I support Fabian's arguments. In my opinion, temporary objects > >> should > >>>>>> just be an additional layer on top of the regular catalog/database > >>>>>> lookup logic. Thus, a temporary table or function has always highest > >>>>>> precedence and should be stable within the local session. Otherwise > >> it > >>>>>> could magically disappear while someone else is performing > >>> modifications > >>>>>> in the catalog. > >>>>>> > >>>>>> Furthermore, this feature is very useful for prototyping as users > >> can > >>>>>> simply express that a catalog/database is present even through they > >>>>>> might not have access to it currently. > >>>>>> > >>>>>> Regards, > >>>>>> Timo > >>>>>> > >>>>>> > >>>>>> On 30.09.19 14:57, Fabian Hueske wrote: > >>>>>>> Hi all, > >>>>>>> > >>>>>>> Sorry for the late reply. > >>>>>>> > >>>>>>> I think it might lead to confusing situations if temporary > >> functions > >>>> (or > >>>>>>> any temporary db objects for that matter) are bound to the life > >>> cycle > >>>>>> of an > >>>>>>> (external) db/catalog. > >>>>>>> Imaging a situation where you create a temp function in a db in an > >>>>>> external > >>>>>>> catalog and use it but at some point it does not work anymore > >>> because > >>>>>> some > >>>>>>> other dropped the database from the external catalog. > >>>>>>> Shouldn't temporary objects be only controlled by the owner of a > >>>>>> session? > >>>>>>> I agree that creating temp objects in non-existing db/catalogs > >>> sounds > >>>> a > >>>>>> bit > >>>>>>> strange, but IMO the opposite (the db/catalog must exist for a > >> temp > >>>>>>> function to be created/exist) can have significant implications > >> like > >>>> the > >>>>>>> one I described. > >>>>>>> I think it would be quite easy for users to understand that > >>> temporary > >>>>>>> objects are solely owned by them (and their session). > >>>>>>> The problem of listing temporary objects could be solved by > >> adding a > >>>> ALL > >>>>>>> [TEMPORARY] clause: > >>>>>>> > >>>>>>> SHOW ALL FUNCTIONS; could show all functions regardless of the > >>>>>>> catalog/database including temporary functions. > >>>>>>> SHOW ALL TEMPORARY FUNCTIONS; could show all temporary functions > >>>>>> regardless > >>>>>>> of the catalog/database. > >>>>>>> > >>>>>>> Best, > >>>>>>> Fabian > >>>>>>> > >>>>>>> Am Sa., 28. Sept. 2019 um 02:21 Uhr schrieb Bowen Li < > >>>>>> bowenl...@gmail.com>: > >>>>>>>> @Dawid, do you have any other concerns? If not, I hope we can > >> close > >>>> the > >>>>>>>> voting. > >>>>>>>> > >>>>>>>> > >>>>>>>> On Thu, Sep 26, 2019 at 8:14 PM Rui Li <lirui.fu...@gmail.com> > >>>> wrote: > >>>>>>>>> I'm not sure how much benefit #1 can bring us. If users just > >> want > >>> to > >>>>>> try > >>>>>>>>> out temporary functions, they can create temporary system > >>> functions > >>>>>> which > >>>>>>>>> don't require a catalog/DB. IIUC, the main reason why we allow > >>>>>> temporary > >>>>>>>>> catalog function is to let users override permanent catalog > >>>> functions. > >>>>>>>>> Therefore a temporary function in a non-existing catalog won't > >>> serve > >>>>>> that > >>>>>>>>> purpose. Besides, each session is provided with a default > >> catalog > >>>> and > >>>>>> DB. > >>>>>>>>> So even if users simply want to create some catalog functions > >> they > >>>> can > >>>>>>>>> forget about after the session, wouldn't the default catalog/DB > >> be > >>>>>> enough > >>>>>>>>> for such experiments? > >>>>>>>>> > >>>>>>>>> On Thu, Sep 26, 2019 at 4:38 AM Bowen Li <bowenl...@gmail.com> > >>>> wrote: > >>>>>>>>>> Re 1) As described in the FLIP, a temp function lookup will > >> first > >>>>>> make > >>>>>>>>> sure > >>>>>>>>>> the db exists. If the db doesn't exist, a lazy drop is > >> triggered > >>> to > >>>>>>>>> remove > >>>>>>>>>> that temp function. > >>>>>>>>>> > >>>>>>>>>> I agree Hive doesn't handle it consistently, and we are not > >>> copying > >>>>>>>> Hive. > >>>>>>>>>> IMHO, allowing registering temp functions in nonexistent > >>> catalog/db > >>>>>> is > >>>>>>>>>> hacky and problematic. For instance, "SHOW FUNCTIONS" would > >> list > >>>>>> system > >>>>>>>>>> functions and functions in the current catalog/db, since users > >>>> cannot > >>>>>>>>>> designate a nonexistent catalog/db as current ones, how can > >> they > >>>> list > >>>>>>>>>> functions in nonexistent catalog/db? They may end up never > >>> knowing > >>>>>> what > >>>>>>>>>> temp functions they've created unless trying out with queries > >> or > >>> we > >>>>>>>>>> introducing some more nonstandard SQL statements. The same > >>> applies > >>>> to > >>>>>>>>> other > >>>>>>>>>> temp objects like temp tables. > >>>>>>>>>> > >>>>>>>>>> Re 2) A standalone FunctionIdentifier sounds good to me > >>>>>>>>>> > >>>>>>>>>> On Wed, Sep 25, 2019 at 4:46 AM Dawid Wysakowicz < > >>>>>>>>>> wysakowicz.da...@gmail.com> > >>>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>>> Ad. 1 > >>>>>>>>>>> I wouldn't say it is hacky. > >>>>>>>>>>> Moreover, how do you want ensure that the dB always exists > >> when > >>> a > >>>>>>>>>> temporary > >>>>>>>>>>> object is used?( in this particular case function). Do you > >> want > >>> to > >>>>>>>>> query > >>>>>>>>>>> for the database existence whenever e.g a temporary function > >> is > >>>>>>>> used? I > >>>>>>>>>>> think important aspect here is that the database can be > >> dropped > >>>> from > >>>>>>>>>>> external system, not just flink or a different flink session. > >>>>>>>>>>> > >>>>>>>>>>> E.g in case of hive, you cannot create a temporary table in a > >>>>>>>> database > >>>>>>>>>> that > >>>>>>>>>>> does not exist, that's true. But if you create a temporary > >> table > >>>> in > >>>>>> a > >>>>>>>>>>> database and drop that database from a different session, you > >>> can > >>>>>>>> still > >>>>>>>>>>> query the previously created temporary table from the original > >>>>>>>> session. > >>>>>>>>>> It > >>>>>>>>>>> does not sound like a consistent behaviour to me. Why don't we > >>>> make > >>>>>>>>> this > >>>>>>>>>>> behaviour of not binding a temporary objects to the lifetime > >> of > >>> a > >>>>>>>>>> database > >>>>>>>>>>> explicit as part of the temporary objects contract? In the end > >>>> they > >>>>>>>>> exist > >>>>>>>>>>> in different layers. Permanent objects & databases in a > >> catalog > >>>> (in > >>>>>>>>> case > >>>>>>>>>> of > >>>>>>>>>>> hive megastore) whereas temporary objects in flink sessions. > >>>> That's > >>>>>>>>> also > >>>>>>>>>>> true for the original hive client. The temporary objects live > >> in > >>>> the > >>>>>>>>> hive > >>>>>>>>>>> client whereas databases are created in the metastore. > >>>>>>>>>>> > >>>>>>>>>>> Ad.2 > >>>>>>>>>>> I'm open for suggestions here. The one thing I wanted to > >> achieve > >>>>>> here > >>>>>>>>> is > >>>>>>>>>> so > >>>>>>>>>>> that we do not change the contract of ObjectIdentifier. One > >>>>>> important > >>>>>>>>>> thing > >>>>>>>>>>> to remember here is that we need the function identifier to be > >>>> part > >>>>>>>> of > >>>>>>>>>> the > >>>>>>>>>>> FunctionDefinition object and not only as the result of the > >>>> function > >>>>>>>>>>> lookup. At some point we want to be able to store > >>> QueryOperations > >>>> in > >>>>>>>>> the > >>>>>>>>>>> catalogs. They can contain function calls within which we need > >>> to > >>>>>>>> have > >>>>>>>>>> the > >>>>>>>>>>> identifier. > >>>>>>>>>>> > >>>>>>>>>>> I agree my initial suggestion is over complicated. How about > >> we > >>>> have > >>>>>>>>> just > >>>>>>>>>>> the FunctionIdentifier as top level class without making the > >>>>>>>>>>> ObjectIdentifier extend from it? I think it's pretty much the > >>> same > >>>>>>>> what > >>>>>>>>>> you > >>>>>>>>>>> suggested. The only difference is that it would be a top level > >>>> class > >>>>>>>>>> with a > >>>>>>>>>>> more descriptive name. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> On Wed, 25 Sep 2019, 13:57 Bowen Li, <bowenl...@gmail.com> > >>> wrote: > >>>>>>>>>>>> Sorry, I missed some parts of the solution. The complete > >>>>>>>> alternative > >>>>>>>>> is > >>>>>>>>>>> the > >>>>>>>>>>>> following, basically having separate APIs in FunctionLookup > >> for > >>>>>>>>>> ambiguous > >>>>>>>>>>>> and precise function lookup since planner is able to tell > >> which > >>>> API > >>>>>>>>> to > >>>>>>>>>>> call > >>>>>>>>>>>> with parsed queries, and have a unified result: > >>>>>>>>>>>> > >>>>>>>>>>>> ``` > >>>>>>>>>>>> class FunctionLookup { > >>>>>>>>>>>> > >>>>>>>>>>>> Optional<Result> lookupAmbiguousFunction(String name); > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> Optional<Result> lookupPreciseFunction(ObjectIdentifier oi); > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> class Result { > >>>>>>>>>>>> Optional<ObjectIdentifier> getObjectIdentifier() { ... > >> } > >>>>>>>>>>>> String getName() { ... } > >>>>>>>>>>>> // ... > >>>>>>>>>>>> } > >>>>>>>>>>>> > >>>>>>>>>>>> } > >>>>>>>>>>>> ``` > >>>>>>>>>>>> > >>>>>>>>>>>> Thanks, > >>>>>>>>>>>> Bowen > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> On Tue, Sep 24, 2019 at 9:42 PM Bowen Li < > >> bowenl...@gmail.com> > >>>>>>>>> wrote: > >>>>>>>>>>>>> Hi Dawid, > >>>>>>>>>>>>> > >>>>>>>>>>>>> Re 1): I agree making it easy for users to run experiments > >> is > >>>>>>>>>>> important. > >>>>>>>>>>>>> However, I'm not sure allowing users to register temp > >>> functions > >>>>>>>> in > >>>>>>>>>>>>> nonexistent catalog/db is the optimal way. It seems a bit > >>> hacky, > >>>>>>>>> and > >>>>>>>>>>>> breaks > >>>>>>>>>>>>> the current contract between Flink and users that catalog/db > >>>> must > >>>>>>>>> be > >>>>>>>>>>>> valid > >>>>>>>>>>>>> in order to operate on. > >>>>>>>>>>>>> > >>>>>>>>>>>>> How about we instead focus on making it convenient to create > >>>>>>>>>> catalogs? > >>>>>>>>>>>>> Users actually can already do it with ease via program or > >> SQL > >>>> CLI > >>>>>>>>>> yaml > >>>>>>>>>>>> file > >>>>>>>>>>>>> for an in-memory catalog which has neither extra dependency > >>> nor > >>>>>>>>>>> external > >>>>>>>>>>>>> connections. What we can further improve is DDL for > >> catalogs, > >>>>>>>> and I > >>>>>>>>>>>> raised > >>>>>>>>>>>>> it in discussion of [FLIP 69 - Flink SQL DDL Enhancement] > >>> driven > >>>>>>>> by > >>>>>>>>>>> Terry > >>>>>>>>>>>>> now. > >>>>>>>>>>>>> > >>>>>>>>>>>>> In that case, if users would like to experiment via SQL, > >> they > >>>> can > >>>>>>>>>>> easily > >>>>>>>>>>>>> create an in memory catalog/database using DDL, then play > >> with > >>>>>>>> temp > >>>>>>>>>>>>> functions. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Re 2): For the assumption, IIUIC, function ObjectIdentifier > >>> has > >>>>>>>> not > >>>>>>>>>>> been > >>>>>>>>>>>>> resolved when stack call reaches > >>>>>>>> FunctionCatalog#lookupFunction(), > >>>>>>>>>> but > >>>>>>>>>>>> only > >>>>>>>>>>>>> been parsed? > >>>>>>>>>>>>> > >>>>>>>>>>>>> I agree keeping ObjectIdentifier as-is would be good. I'm ok > >>>> with > >>>>>>>>> the > >>>>>>>>>>>>> suggested classes, though making ObjectIdentifier a subclass > >>> of > >>>>>>>>>>>>> FunctionIdentifier seem a bit counter intuitive. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Another potentially simpler way is: > >>>>>>>>>>>>> > >>>>>>>>>>>>> ``` > >>>>>>>>>>>>> // in class FunctionLookup > >>>>>>>>>>>>> class Result { > >>>>>>>>>>>>> Optional<ObjectIdentifier> getObjectIdentifier() { > >> ... } > >>>>>>>>>>>>> String getName() { ... } > >>>>>>>>>>>>> ... > >>>>>>>>>>>>> } > >>>>>>>>>>>>> ``` > >>>>>>>>>>>>> > >>>>>>>>>>>>> WDYT? > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Tue, Sep 24, 2019 at 3:41 PM Dawid Wysakowicz < > >>>>>>>>>>>>> wysakowicz.da...@gmail.com> wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>>> Hi, > >>>>>>>>>>>>>> I really like the flip and think it clarifies important > >>> aspects > >>>>>>>> of > >>>>>>>>>> the > >>>>>>>>>>>>>> system. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> I have two, I hope small suggestions, which will not take > >>> much > >>>>>>>>> time > >>>>>>>>>> to > >>>>>>>>>>>>>> agree on. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> 1. Could we follow the MySQL approach in regards to the > >>>>>>>> existence > >>>>>>>>> of > >>>>>>>>>>>>>> cat/db > >>>>>>>>>>>>>> for temporary functions? That means not to check it, so > >> e.g. > >>>>>>>> it's > >>>>>>>>>>>> possible > >>>>>>>>>>>>>> to create a temporary function in a database that does not > >>>>>>>> exist. > >>>>>>>>> I > >>>>>>>>>>>> think > >>>>>>>>>>>>>> it's really useful e.g in cases when user wants to perform > >>>>>>>>>> experiments > >>>>>>>>>>>> but > >>>>>>>>>>>>>> does not have access to the db yet or temporarily does not > >>> have > >>>>>>>>>>>> connection > >>>>>>>>>>>>>> to a catalog. > >>>>>>>>>>>>>> 2. Could we not change the ObjectIdentifier? Could we not > >>>> loosen > >>>>>>>>> the > >>>>>>>>>>>>>> requirements for all catalog objects such as tables, views, > >>>>>>>> types > >>>>>>>>>> just > >>>>>>>>>>>> for > >>>>>>>>>>>>>> the functions? It's really important later on from e.g the > >>>>>>>>>>>> serializability > >>>>>>>>>>>>>> perspective. The important aspect of the ObjectIdentifier > >> is > >>>>>>>> that > >>>>>>>>> we > >>>>>>>>>>>> know > >>>>>>>>>>>>>> that it has been resolved. The suggested changes break that > >>>>>>>>>>> assumption. > >>>>>>>>>>>>>> What do you think about adding an interface > >>> FunctionIdentifier > >>>> { > >>>>>>>>>>>>>> String getName(); > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> /** > >>>>>>>>>>>>>> Return 3-part identifier. Empty in case of a built-in > >>>>>>>> function. > >>>>>>>>>>>>>> */ > >>>>>>>>>>>>>> Optional<ObjectIdentifier> getObjectIdentifier() > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> class ObjectIdentifier implements FunctionIdentifier { > >>>>>>>>>>>>>> Optional<ObjectIdentifier> getObjectIdentifier() { > >>>>>>>>>>>>>> return Optional.of(this); > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> class SystemFunctionIdentifier implements > >> FunctionIdentifier > >>>>>>>> {...} > >>>>>>>>>>>>>> WDYT? > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On Wed, 25 Sep 2019, 04:50 Xuefu Z, <usxu...@gmail.com> > >>> wrote: > >>>>>>>>>>>>>>> +1. LGTM > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On Tue, Sep 24, 2019 at 6:09 AM Terry Wang < > >>>>>>>> zjuwa...@gmail.com> > >>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>> +1 > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>> Terry Wang > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> 在 2019年9月24日,上午10:42,Kurt Young <ykt...@gmail.com> 写道: > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> +1 > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>> Kurt > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> On Tue, Sep 24, 2019 at 2:30 AM Bowen Li < > >>>>>>>>> bowenl...@gmail.com > >>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>> Hi all, > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> I'd like to start a voting thread for FLIP-57 [1], > >> which > >>>>>>>>>> we've > >>>>>>>>>>>>>> reached > >>>>>>>>>>>>>>>>>> consensus in [2]. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> This voting will be open for minimum 3 days till 6:30pm > >>>>>>>>> UTC, > >>>>>>>>>>> Sep > >>>>>>>>>>>>>> 26. > >>>>>>>>>>>>>>>>>> Thanks, > >>>>>>>>>>>>>>>>>> Bowen > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> [1] > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >> > https://cwiki.apache.org/confluence/display/FLINK/FLIP-57%3A+Rework+FunctionCatalog > >>>>>>>>>>>>>>>>>> [2] > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >> > http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-FLIP-57-Rework-FunctionCatalog-td32291.html#a32613 > >>>>>>>>>>>>>>> -- > >>>>>>>>>>>>>>> Xuefu Zhang > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> "In Honey We Trust!" > >>>>>>>>>>>>>>> > >>>>>>>>> -- > >>>>>>>>> Best regards! > >>>>>>>>> Rui Li > >>>>>>>>> > >>>>>> > > > >