Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-15 Thread Bowen Li
Hi all, I have updated the doc to reflect the discussion results. I will start a voting thread shortly. Thanks! Bowen On Fri, Oct 11, 2019 at 12:44 AM Timo Walther wrote: > I'm fine with `type` for consistency reasons for now. I hope we will fix > that when we rework the properties design. > >

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-11 Thread Timo Walther
I'm fine with `type` for consistency reasons for now. I hope we will fix that when we rework the properties design. @Bowen: could you update the wiki page? I think we could start a vote, right? Regards, Timo On 11.10.19 04:31, Jark Wu wrote: Hi Timo, I agree that we are going to rework pro

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-10 Thread Jark Wu
Hi Timo, I agree that we are going to rework properties soon. But we may come up with a better name or a better way than "kind" when the proposal is started and more people involved. On the other hand, reworking properties should be a compatible way. So I think it's fine to use "type" for now (kee

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-10 Thread Timo Walther
Hi Jark, restricting one module instance per kind sounds good to me. Modules can implement hashCode/equals and we can perform the check you metioned. The equals() method can determine what "same kind" means. Correct me if I'm wrong, but we wanted to perform a big properties rework soonish an

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-10 Thread Jark Wu
Hi Timo, Thanks for the explanation, it makes sense to me. So at least, we can have a validation to restrict one module instance per type in the first version. Regarding to "type" vs "kind", could we use "datatype" keyword to refer data types exclusively in the future? This can avoid the conflict

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-10 Thread Timo Walther
Hi Jark, we had a long offline discussion yesterday where we considered all options again. The reasons why we decided for the updated design that Bowen suggested: - Both Dawid and Xuefu argued that in the old design "kind" has binary meanings. I agree here. - Compared to other SQL concepts s

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-09 Thread Jark Wu
Hi Xuefu, If there is only one instance per type, then what's the "name" used for? Could we remove it and only keep "type" or "kind" to identify modules? Best, Jark On Thu, 10 Oct 2019 at 11:21, Xuefu Z wrote: > Jark has a good point. However, I think validation logic can put in place > to res

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-09 Thread Xuefu Z
Jark has a good point. However, I think validation logic can put in place to restrict one instance per type. Maybe the doc needs to be specific on this. Thanks, Xuefu On Wed, Oct 9, 2019 at 7:41 PM Jark Wu wrote: > Thanks Bowen for the updating. > > I have some different opinions on the change.

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-09 Thread Jark Wu
Thanks Bowen for the updating. I have some different opinions on the change. IIUC, in the previous design, the "name" is also the "id" or "type" to identify which module to load. Which means we can only load one instance of a module. In the new design, the "name" is just an alias to the module ins

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-09 Thread Bowen Li
Thanks everyone for your review. After discussing with Timo and Dawid offline, as well as incorporating feedback from Xuefu and Jark on mailing list, I decided to make a few critical changes to the proposal. - renamed the keyword "type" to "kind". The community has plan to have "type" keyword in

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-07 Thread Jark Wu
Hi Bowen, Thanks for the proposal. I have two thoughts: 1) Regarding to "loadModule", how about tableEnv.loadModule("xxx" [, propertiesMap]); tableEnv.unloadModule(“xxx”); This makes the API similar to SQL. IMO, instance of Module is not needed and verbose as parameter. And this makes it easier

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-04 Thread Xuefu Z
I agree with Timo that the new table APIs need to be consistent. I'd go further that an name (or id) is needed for module definition in YAML file. In the current design, name is skipped and type has binary meanings. Thanks, Xuefu On Fri, Oct 4, 2019 at 5:24 AM Timo Walther wrote: > Hi everyone,

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-04 Thread Timo Walther
Hi everyone, first, I was also questioning my proposal. But Bowen's proposal of `tEnv.offloadToYaml()` would not work with the current design because we don't know how to serialize a catalog or module into properties. Currently, there is no converter from instance to properties. It is a one w

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-01 Thread Bowen Li
If something like the yaml file is the way to go and achieve such motivation, we would cover that with current design. On Tue, Oct 1, 2019 at 12:05 Bowen Li wrote: > Hi Timo, Dawid, > > I've added the suggested SQL and related changes to TableEnvironment API > and other classes to the google doc

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-01 Thread Bowen Li
Hi Timo, Dawid, I've added the suggested SQL and related changes to TableEnvironment API and other classes to the google doc. Also removed "USE MODULE" and its APIs. Will update FLIP wiki once we have a consensus. W.r.t. descriptor approach, my gut feeling is similar to Dawid's. Besides, I feel y

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-01 Thread Dawid Wysakowicz
Hi Timo, Bowen, Unfortunately I did not have enough time to go through all the suggestions in details so I can not comment on the whole FLIP. I just wanted to give my opinion on the "descriptor approach in loadModule" part. I am not sure if we need it here. We might be overthinking this a bit. It

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-10-01 Thread Timo Walther
Hi Bowen, thanks for your response. Re 2) I also don't have a better approach for this issue. It is similar to changing the general TableConfig between two statements. It would be good to add your explanation to the design document. Re 3) It would be interesting to know about which "core" fu

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-09-30 Thread Bowen Li
Hi Timo, Re 1) I agree. I renamed the title to "Extend Core Table System with Pluggable Modules" and all internal references Re 2) First, I'll rename the API to useModules(). The design doesn't forbid users to call useModules() multi times. Objects in modules are loaded on demand instead of eager

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-09-30 Thread Timo Walther
Hi Bowen, thanks for this proposal after our discussion around the FunctionCatalog rework. I like the architecture proposed in the FLIP because it is also based on existing concepts and just slightly modifies the code base. However, I would like to discuss some unanswered questions: 1) Termi

Re: [DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-09-19 Thread Bowen Li
Thanks everyone for your feedback. I've converted it to a FLIP wiki [1]. Please take another look. If there's no more concerns, I'd like to start a voting thread for it. Thanks [1] https://cwiki.apache.org/confluence/display/FLINK/FLIP-68%3A+Extend+Core+Table+System+with+Modular+Plugins On T

[DISCUSS] FLIP-68: Extend Core Table System with Modular Plugins

2019-09-17 Thread Bowen Li
Hi devs, We'd like to kick off a conversation on "FLIP-68: Extend Core Table System with Modular Plugins" [1]. The modular approach was raised in discussion of how to support Hive built-in functions in FLIP-57 [2]. As we discussed and looked deeper, we think it’s a good opportunity to broaden th