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

Add user_defined_sql_planners(..) to FunctionRegistry #11296

Merged
merged 4 commits into from
Jul 7, 2024
Merged

Conversation

Omega359
Copy link
Contributor

@Omega359 Omega359 commented Jul 5, 2024

Which issue does this PR close?

Closes #11294

Rationale for this change

Adding the ability to retrieve the list of registered user defined sql functions.

What changes are included in this PR?

Code, test.

Are these changes tested?

A simple test was added to verify api returns something.

Are there any user-facing changes?

FunctionRegistry trait was ammended.

@github-actions github-actions bot added logical-expr Logical plan and expressions core Core DataFusion crate labels Jul 5, 2024
@Omega359 Omega359 marked this pull request as ready for review July 5, 2024 22:36
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me -- thank you @Omega359

However what do you think about using the name expr_planners rather than user_defined_sql_planners #11304 ?

I realize this PR is consistent with the current names, but as the API is expanding so it is probably a good ideae to ensure the nemes are good

@Omega359
Copy link
Contributor Author

Omega359 commented Jul 6, 2024

I'm ok with that as long as the register function neme (and I suppose deregister in the future) mirrors the naming scheme. I can file a followup issue to have that renamed.

@Omega359
Copy link
Contributor Author

Omega359 commented Jul 6, 2024

Rename update pushed.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Omega359

@alamb alamb merged commit 9f8ba6a into apache:main Jul 7, 2024
23 checks passed
findepi pushed a commit to findepi/datafusion that referenced this pull request Jul 16, 2024
* Add user_defined_sql_planners(..) to FunctionRegistry

* Adding simple test for user_defined_sql_planners

* Renamed user_defined_sql_planners to expr_planners
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate logical-expr Logical plan and expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add supporting functions for user defined sql planners to FunctionRegistry
2 participants