+1 to Jark's proposal
To make it clearer, will `module#getFunctionDefinition()` return empty
suppose the module is loaded but not enabled?
Best,
Jane
On Mon, Feb 1, 2021 at 10:02 PM Timo Walther <twal...@apache.org> wrote:
> +1 to Jark's proposal
>
> I like the difference between just loading and actually enabling these
> modules.
>
> @Rui: I would use the same behavior as catalogs here. You cannot `USE` a
> catalog without creating it before.
>
> Another question is whether a LOAD operation also adds the module to the
> enabled list by default?
>
> Regards,
> Timo
>
> On 01.02.21 13:52, Rui Li wrote:
> > If `USE MODULES` implies unloading modules that are not listed, does it
> > also imply loading modules that are not previously loaded, especially
> since
> > we're mapping modules by name now?
> >
> > On Mon, Feb 1, 2021 at 8:20 PM Jark Wu <imj...@gmail.com> wrote:
> >
> >> I agree with Timo that the USE implies the specified modules are in use
> in
> >> the specified order and others are not used.
> >> This would be easier to know what's the result list and order after the
> USE
> >> statement.
> >> That means: if current modules in order are x, y, z. And `USE MODULES
> z, y`
> >> means current modules in order are z, y.
> >>
> >> But I would like to not unload the unmentioned modules in the USE
> >> statement. Because it seems strange that USE
> >> will implicitly remove modules. In the above example, the user may type
> the
> >> wrong modules list using USE by mistake
> >> and would like to declare the list again, the user has to create the
> >> module again with some properties he may don't know. Therefore, I
> propose
> >> the USE statement just specifies the current module lists and doesn't
> >> unload modules.
> >> Besides that, we may need a new syntax to list all the modules including
> >> not used but loaded.
> >> We can introduce SHOW FULL MODULES for this purpose with an additional
> >> `used` column.
> >>
> >> For example:
> >>
> >> Flink SQL> list modules:
> >> -----------
> >> | modules |
> >> -----------
> >> | x |
> >> | y |
> >> | z |
> >> -----------
> >> Flink SQL> USE MODULES z, y;
> >> Flink SQL> show modules:
> >> -----------
> >> | modules |
> >> -----------
> >> | z |
> >> | y |
> >> -----------
> >> Flink SQL> show FULL modules;
> >> -------------------
> >> | modules | used |
> >> -------------------
> >> | z | true |
> >> | y | true |
> >> | x | false |
> >> -------------------
> >> Flink SQL> USE MODULES z, y, x;
> >> Flink SQL> show modules;
> >> -----------
> >> | modules |
> >> -----------
> >> | z |
> >> | y |
> >> | x |
> >> -----------
> >>
> >> What do you think?
> >>
> >> Best,
> >> Jark
> >>
> >> On Mon, 1 Feb 2021 at 19:02, Jane Chan <qingyue....@gmail.com> wrote:
> >>
> >>> Hi Timo, thanks for the discussion.
> >>>
> >>> It seems to reach an agreement regarding #3 that <1> Module name should
> >>> better be a simple identifier rather than a string literal. <2>
> Property
> >>> `type` is redundant and should be removed, and mapping will rely on the
> >>> module name because loading a module multiple times just using a
> >> different
> >>> module name doesn't make much sense. <3> We should migrate to the newer
> >> API
> >>> rather than the deprecated `TableFactory` class.
> >>>
> >>> Regarding #1, I think the point lies in whether changing the resolution
> >>> order implies an `unload` operation explicitly (i.e., users could sense
> >>> it). What do others think?
> >>>
> >>> Best,
> >>> Jane
> >>>
> >>> On Mon, Feb 1, 2021 at 6:41 PM Timo Walther <twal...@apache.org>
> wrote:
> >>>
> >>>> IMHO I would rather unload the not mentioned modules. The statement
> >>>> expresses `USE` that implicilty implies that the other modules are
> "not
> >>>> used". What do others think?
> >>>>
> >>>> Regards,
> >>>> Timo
> >>>>
> >>>>
> >>>> On 01.02.21 11:28, Jane Chan wrote:
> >>>>> Hi Jark and Rui,
> >>>>>
> >>>>> Thanks for the discussions.
> >>>>>
> >>>>> Regarding #1, I'm fine with `USE MODULES` syntax, and
> >>>>>
> >>>>>> It can be interpreted as "setting the current order of modules",
> >> which
> >>>> is
> >>>>>> similar to "setting the current catalog" for `USE CATALOG`.
> >>>>>>
> >>>>> I would like to confirm that the unmentioned modules remain in the
> >> same
> >>>>> relative order? E.g., if there are three loaded modules `X`, `Y`,
> >> `Z`,
> >>>> then
> >>>>> `USE MODULES Y, Z` means shifting the order to `Y`, `Z`, `X`.
> >>>>>
> >>>>> Regarding #3, I'm fine with mapping modules purely by name, and I
> >> think
> >>>>> Jark raised a good point on making the module name a simple
> >> identifier
> >>>>> instead of a string literal. For backward compatibility, since we
> >>> haven't
> >>>>> supported this syntax yet, the affected users are those who defined
> >>>> modules
> >>>>> in the YAML configuration file. Maybe we can eliminate the 'type'
> >> from
> >>>> the
> >>>>> 'requiredContext' to make it optional. Thus the proposed mapping
> >>>> mechanism
> >>>>> could use the module name to lookup the suitable factory, and in the
> >>>>> meanwhile updating documentation to encourage users to simplify their
> >>>> YAML
> >>>>> configuration. And in the long run, we can deprecate the 'type'.
> >>>>>
> >>>>> Best,
> >>>>> Jane
> >>>>>
> >>>>> On Mon, Feb 1, 2021 at 4:19 PM Rui Li <lirui.fu...@gmail.com> wrote:
> >>>>>
> >>>>>> Thanks Jane for starting the discussion.
> >>>>>>
> >>>>>> Regarding #1, I also prefer `USE MODULES` syntax. It can be
> >>> interpreted
> >>>> as
> >>>>>> "setting the current order of modules", which is similar to "setting
> >>> the
> >>>>>> current catalog" for `USE CATALOG`.
> >>>>>>
> >>>>>> Regarding #3, I'm fine to map modules purely by name because I think
> >>> it
> >>>>>> satisfies all the use cases we have at hand. But I guess we need to
> >>> make
> >>>>>> sure we're backward compatible, i.e. users don't need to change
> >> their
> >>>> yaml
> >>>>>> files to configure the modules.
> >>>>>>
> >>>>>> On Mon, Feb 1, 2021 at 3:10 PM Jark Wu <imj...@gmail.com> wrote:
> >>>>>>
> >>>>>>> Thanks Jane for the summary and starting the discussion in the
> >>> mailing
> >>>>>>> list.
> >>>>>>>
> >>>>>>> Here are my thoughts:
> >>>>>>>
> >>>>>>> 1) syntax to reorder modules
> >>>>>>> I agree with Rui Li it would be quite useful if we can have some
> >>> syntax
> >>>>>> to
> >>>>>>> reorder modules.
> >>>>>>> I slightly prefer `USE MODULES x, y, z` than `RELOAD MODULES x, y,
> >>> z`,
> >>>>>>> because USE has a more sense of effective and specifying ordering,
> >>> than
> >>>>>>> RELOAD.
> >>>>>>> From my feeling, RELOAD just means we unregister and register
> >> x,y,z
> >>>>>> modules
> >>>>>>> again,
> >>>>>>> it sounds like other registered modules are still in use and in the
> >>>>>> order.
> >>>>>>>
> >>>>>>> 3) mapping modules purely by name
> >>>>>>> This can definitely improve the usability of loading modules,
> >> because
> >>>>>>> the 'type=' property
> >>>>>>> looks really redundant. We can think of this as a syntax sugar that
> >>> the
> >>>>>>> default type value is the module name.
> >>>>>>> And we can support to specify 'type=' property in the future to
> >> allow
> >>>>>>> multiple modules for one module type.
> >>>>>>>
> >>>>>>> Besides, I would like to mention one more change, that the module
> >>> name
> >>>>>>> proposed in FLIP-68 is a string literal.
> >>>>>>> But I think we are all on the same page to change it into a simple
> >>>>>>> (non-compound) identifier.
> >>>>>>>
> >>>>>>> LOAD/UNLOAD MODULE 'core'
> >>>>>>> ==>
> >>>>>>> LOAD/UNLOAD MODULE core
> >>>>>>>
> >>>>>>>
> >>>>>>> Best,
> >>>>>>> Jark
> >>>>>>>
> >>>>>>>
> >>>>>>> On Sat, 30 Jan 2021 at 04:00, Jane Chan <qingyue....@gmail.com>
> >>> wrote:
> >>>>>>>
> >>>>>>>> Hi everyone,
> >>>>>>>>
> >>>>>>>> I would like to start a discussion on FLINK-21045 [1] about
> >>> supporting
> >>>>>>>> `LOAD MODULE` and `UNLOAD MODULE` SQL syntax. It's first proposed
> >> by
> >>>>>>>> FLIP-68 [2] as following.
> >>>>>>>>
> >>>>>>>> -- load a module with the given name and append it to the end of
> >> the
> >>>>>>> module
> >>>>>>>> list
> >>>>>>>> LOAD MODULE 'name' [WITH ('type'='xxx', 'prop'='myProp', ...)]
> >>>>>>>>
> >>>>>>>> --unload a module by name from the module list and other modules
> >>>> remain
> >>>>>>> in
> >>>>>>>> the same relative positions
> >>>>>>>> UNLOAD MODULE 'name'
> >>>>>>>>
> >>>>>>>> After a round of discussion on the Jira ticket, it seems some
> >>>>>> unanswered
> >>>>>>>> questions need more opinions and suggestions.
> >>>>>>>>
> >>>>>>>> 1. The way to redefine resolution order easily
> >>>>>>>>
> >>>>>>>> Rui Li suggested introducing `USE MODULES` and adding
> similar
> >>>>>>>> functionality to the API because
> >>>>>>>>
> >>>>>>>>> 1) It's very tedious to unload old modules just to reorder
> >> them.
> >>>>>>>>
> >>>>>>>> 2) Users may not even know how to "re-load" an old module if it
> >>> was
> >>>>>> not
> >>>>>>>>> initially loaded by the user, e.g. don't know which type to use.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Jane Chan wondered that module is not like the catalog which
> >>> has
> >>>> a
> >>>>>>>> concept of namespace could specify, and `USE` sounds like a
> >>>>>>>> mutual-exclusive concept.
> >>>>>>>> Maybe `RELOAD MODULES` can express upgrading the priority of
> >>> the
> >>>>>>> loaded
> >>>>>>>> module(s).
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> 2. `LOAD/UNLOAD MODULE` v.s. `CREATE/DROP MODULE` syntax
> >>>>>>>> Jark Wu and Nicholas Jiang proposed to use `CREATE/DROP
> >> MODULE`
> >>>>>>> instead
> >>>>>>>> of `LOAD/UNLOAD MODULE` because
> >>>>>>>>
> >>>>>>>>> 1) From a pure SQL user's perspective, maybe `CREATE MODULE +
> >> USE
> >>>>>>>> MODULE`
> >>>>>>>>> is easier to use rather than `LOAD/UNLOAD`.
> >>>>>>>>> 2) This will be very similar to what the catalog used now.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Timo Walther would rather stick to the agreed design because
> >>>>>>>> loading/unloading modules is a concept known from kernels etc.
> >>>>>>>>
> >>>>>>>> 3. Simplify the module design by mapping modules purely by name
> >>>>>>>>
> >>>>>>>> LOAD MODULE geo_utils
> >>>>>>>> LOAD MODULE hive WITH ('version'='2.1') -- no dedicated
> >>>>>>> 'type='/'module='
> >>>>>>>> but allow only 1 module to be loaded parameterized
> >>>>>>>> UNLOAD hive
> >>>>>>>> USE MODULES hive, core
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Please find more details in the reference link. Looking forward to
> >>>> your
> >>>>>>>> feedback.
> >>>>>>>>
> >>>>>>>> [1] https://issues.apache.org/jira/browse/FLINK-21045#
> >>>>>>>> <
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>
> >>>
> >>
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-68%3A+Extend+Core+Table+System+with+Pluggable+Modules
> >>>>>>>>>
> >>>>>>>> [2]
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>
> >>>
> >>
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-68%3A+Extend+Core+Table+System+with+Pluggable+Modules
> >>>>>>>>
> >>>>>>>> Best,
> >>>>>>>> Jane
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> Best regards!
> >>>>>> Rui Li
> >>>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>
> >
> >
>
>
