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

implement string_to_array #7577

Merged
merged 3 commits into from
Sep 18, 2023
Merged

implement string_to_array #7577

merged 3 commits into from
Sep 18, 2023

Conversation

casperhart
Copy link
Contributor

Closes #7555

What changes are included in this PR?

Are these changes tested?

I've included tests in both tests/sql/expr.rs and functions.slt. This duplication may not be necessary, so I'd be happy to remove one.

Are there any user-facing changes?

Yes, the addition of a new sql function is a user facing change. However, I'm not sure how to update the docs, but happy to do so if you can point me in the right direction.

@github-actions github-actions bot added logical-expr Logical plan and expressions physical-expr Physical Expressions core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Sep 16, 2023
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 @casperhart

It is neat to see something like split be added to DataFusion

Yes, the addition of a new sql function is a user facing change. However, I'm not sure how to update the docs, but happy to do so if you can point me in the right direction.

The docs to update are in https://github.com/apache/arrow-datafusion/blob/main/docs/source/user-guide/sql/scalar_functions.md#array-functions

cc @jayzhan211 and @izveigor do either if you have some time to review this PR?

datafusion/sqllogictest/test_files/functions.slt Outdated Show resolved Hide resolved
datafusion/core/tests/sql/expr.rs Outdated Show resolved Hide resolved
datafusion/expr/src/built_in_function.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the core Core DataFusion crate label Sep 17, 2023
Copy link
Contributor

@jayzhan211 jayzhan211 left a comment

Choose a reason for hiding this comment

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

LGTM!

datafusion/sqllogictest/test_files/functions.slt Outdated Show resolved Hide resolved
datafusion/sqllogictest/test_files/functions.slt Outdated Show resolved Hide resolved
Copy link
Contributor

@jayzhan211 jayzhan211 left a comment

Choose a reason for hiding this comment

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

LGTM!

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 @casperhart and @jayzhan211 for the reivew. I think this PR is looking very nice 👨‍🍳 👌

@alamb alamb merged commit 81f33b0 into apache:main Sep 18, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logical-expr Logical plan and expressions physical-expr Physical Expressions sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

string_to_array type function to split a string on a delimiter, returning an array.
3 participants