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
> >>
> >
>
>
