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

refactor: make all udf function impls public #9903

Merged
merged 3 commits into from
Apr 2, 2024
Merged

Conversation

universalmind303
Copy link
Contributor

Which issue does this PR close?

Closes #9900

Rationale for this change

What changes are included in this PR?

this just makes all of the structs implementing ScalarUDFImpl publicly accessible

Are these changes tested?

Are there any user-facing changes?

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

I wonder if you want to try and add some sort of test to ensure these are all pub somehow? If not, I predict that as we port over additional functions some will get missed

@@ -54,11 +54,17 @@ make_udf_function!(ArrayHasAny,
);

#[derive(Debug)]
pub(super) struct ArrayHas {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that all the arrays function are public, do you plan to public the rest of them too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this should include all of functions from functions and functions-array crates at the time of authoring.

@alamb
Copy link
Contributor

alamb commented Apr 2, 2024

Thanks @universalmind303. BTW @viirya is discussing adding a way to instantiate only some of the UDFs here #9892

@alamb alamb merged commit 760500b into apache:main Apr 2, 2024
24 checks passed
@alamb
Copy link
Contributor

alamb commented Apr 2, 2024

I merged this branch up locally to main and verified clippy and tests passed. 🚀

alamb pushed a commit to alamb/datafusion that referenced this pull request Apr 16, 2024
* refactor: make all udf function impls public

* clippy
alamb added a commit that referenced this pull request Apr 17, 2024
* refactor: make all udf function impls public

* clippy

Co-authored-by: universalmind303 <cory.grinstead@gmail.com>
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.

make udf structs public
3 participants