Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enums support #2: provide enums via FunctionNamespaceManagers #14828

Closed

Conversation

daniel-ohayon
Copy link
Contributor

@daniel-ohayon daniel-ohayon commented Jul 10, 2020

This PR follows #14728
Commits start from "provide types from FunctionNamespaceMgr" (seems like we can't do stacked PRs?)

Design

We want plugins to be able to provide enum definitions. However, the current getTypes method on the Plugin interface is not sufficient, because these types are only loaded at server startup time. Instead, if people modify existing enums, or register new ones, we want those to be usable in queries immediately, without the need to reset the cluster.

These requirements are similar to those of user-defined SQL functions, which is why it makes sense to expose these types via the FunctionNamespaceManager (FNM) interface. Another argument in favor of doing this is that if users want to register UDFs that use these custom types in their signature, we don't end up with dependencies between different plugins/FNMs: the same FNM can provide the enums and the functions.

Implementation

The gist of the change is in TypeRegistry::getType, where we look up a type signature in the available FNMs if it can't be found in our built-in list of types.
The rest of the code is refactoring the plumbing to load FNMs inside a TypeRegistry.

@daniel-ohayon daniel-ohayon assigned rongrong and unassigned rongrong Jul 10, 2020
@daniel-ohayon daniel-ohayon requested a review from rongrong July 10, 2020 23:16
@daniel-ohayon daniel-ohayon force-pushed the dohayon-enum-plugins branch 2 times, most recently from a6ca5ce to 013fdba Compare August 19, 2020 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants