Thanks for starting the discussion Jane.

I'm fine with using `USE` for reordering the modules.

I agree with Jark to not use a string literal for the module name but an identifer.

However, to simplify the design I would completely remove the `type=` property because having multiple ways of defining the same thing might be confusing without providing additional benefits. I also think that users should not be able to load the same module multiple times.

Regarding Rui's comment, the YAML file should not be affected by this change and we can leave this part of the API untouched. We need to update the `ModuleFactory` anyways because it still uses the deprecated `TableFactory` class.

Regards,
Timo

On 01.02.21 09:18, Rui Li 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





Reply via email to