Skip to content

Implement Aliases for ScalarUDF #8360

New issue

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

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

Already on GitHub? # to your account

Merged
merged 2 commits into from
Nov 30, 2023
Merged

Conversation

Veeupup
Copy link
Contributor

@Veeupup Veeupup commented Nov 29, 2023

Which issue does this PR close?

Closes #8348

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Signed-off-by: veeupup <code@tanweime.com>
@github-actions github-actions bot added logical-expr Logical plan and expressions core Core DataFusion crate labels Nov 29, 2023
@Veeupup
Copy link
Contributor Author

Veeupup commented Nov 29, 2023

@alamb PTAL : )

I'm considering that whether we need to provide a helper function create_udf_with_aliaes

@@ -341,6 +341,43 @@ async fn case_sensitive_identifiers_user_defined_functions() -> Result<()> {
Ok(())
}

#[tokio::test]
async fn test_user_defined_functions_with_alias() -> Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe useful to test again the case where you create a udf1 with an alias and then a different udf with the same alias? I don't know what's the expected behavior there

Copy link
Contributor Author

@Veeupup Veeupup Nov 30, 2023

Choose a reason for hiding this comment

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

the behavior will be like HashMap::insert, the old value of the same alias will be updated with the new udf

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 @Veeupup -- this is very nice -- thank you. I have a few style and documentation suggestions, but I also think they could be done as a follow on PR.

Thank you @edmondop for the review and excellent question

Signed-off-by: veeupup <code@tanweime.com>
@Veeupup Veeupup force-pushed the alias_for_scalar_udf branch from 71ca93a to f495f77 Compare November 30, 2023 01:08
@Veeupup
Copy link
Contributor Author

Veeupup commented Nov 30, 2023

thank you for your review! @alamb @edmondop comments are resolved : )

@alamb alamb merged commit d45cf00 into apache:main Nov 30, 2023
@alamb
Copy link
Contributor

alamb commented Nov 30, 2023

Thank you @Veeupup and @edmondop -- we are really cranking now!

# for free to join this conversation on GitHub. Already have an account? # 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.

Implement Aliases for ScalarUDF
3 participants