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 contains function, and support in datafusion substrait consumer #10879

Merged
merged 6 commits into from
Jun 15, 2024

Conversation

Lordworms
Copy link
Contributor

Which issue does this PR close?

Closes #10861

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added sqllogictest SQL Logic Tests (.slt) substrait labels Jun 11, 2024
@Lordworms Lordworms marked this pull request as ready for review June 11, 2024 23:35
) -> Result<ArrayRef, DataFusionError> {
let mod_str = as_generic_string_array::<T>(&args[0])?;
let match_str = as_generic_string_array::<T>(&args[1])?;
let res = arrow::compute::kernels::comparison::regexp_is_match_utf8(
Copy link
Member

@waynexia waynexia Jun 12, 2024

Choose a reason for hiding this comment

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

Do we need to escape search string as it's used in regexp? Wondering what's the result of contains("abcdefg", ".*")

Copy link
Contributor

Choose a reason for hiding this comment

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

@Lordworms can you possibly respond to @waynexia 's comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

To keep the review queue clear, I filed #10929 to follow up on this item and will merge this PR in

Copy link
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

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

Thanks @Lordworms, this looks good to me in general

@alamb alamb changed the title Support for contains function in datafusion substrait consumer Add contains function, and support in in datafusion substrait consumer Jun 12, 2024
@alamb alamb changed the title Add contains function, and support in in datafusion substrait consumer Add contains function, and support in datafusion substrait consumer Jun 12, 2024
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.

Can we also please add an entry to the documentation about this new function? https://datafusion.apache.org/user-guide/sql/scalar_functions.html#scalar-functions

@@ -149,6 +149,9 @@ pub mod expr_fn {
),(
uuid,
"returns uuid v4 as a string value",
), (
contains,
"Return true if search_string is found within string.",
Copy link
Contributor

@alamb alamb Jun 12, 2024

Choose a reason for hiding this comment

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

I think we should also mention here that search_string is treated like a regex if that is the intention

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

datafusion/functions/src/string/contains.rs Show resolved Hide resolved
@Lordworms
Copy link
Contributor Author

Lordworms commented Jun 13, 2024

Can we also please add an entry to the documentation about this new function? https://datafusion.apache.org/user-guide/sql/scalar_functions.html#scalar-functions

Sure, sorry for the ignorance

@Weijun-H
Copy link
Member

Please add an entry here

## String Functions

@alamb
Copy link
Contributor

alamb commented Jun 14, 2024

The CI failure seems unrelated to the code in this PR: https://github.com/apache/datafusion/actions/runs/9510703358?pr=10879

@alamb
Copy link
Contributor

alamb commented Jun 15, 2024

Thanks again @waynexia @Lordworms and @Weijun-H !

@alamb alamb merged commit 87aea14 into apache:main Jun 15, 2024
24 checks passed
devinjdangelo pushed a commit to devinjdangelo/arrow-datafusion that referenced this pull request Jun 16, 2024
…pache#10879)

* adding new function contains

* adding substrait test

* adding doc

* adding doc

* Update docs/source/user-guide/sql/scalar_functions.md

Co-authored-by: Alex Huang <[email protected]>

* adding entry

---------

Co-authored-by: Alex Huang <[email protected]>
alamb added a commit that referenced this pull request Jun 28, 2024
…11060)

* wip create and register ext file types with session

* Add contains function, and support in datafusion substrait consumer (#10879)

* adding new function contains

* adding substrait test

* adding doc

* adding doc

* Update docs/source/user-guide/sql/scalar_functions.md

Co-authored-by: Alex Huang <[email protected]>

* adding entry

---------

Co-authored-by: Alex Huang <[email protected]>

* logical planning updated

* compiling

* removing filetype enum

* compiling

* working on tests

* fix some tests

* test fixes

* cli fix

* cli fmt

* Update datafusion/core/src/datasource/file_format/mod.rs

Co-authored-by: Andrew Lamb <[email protected]>

* Update datafusion/core/src/execution/session_state.rs

Co-authored-by: Andrew Lamb <[email protected]>

* review comments

* review comments

* review comments

* typo fix

* fmt

* fix err log style

* fmt

---------

Co-authored-by: Lordworms <[email protected]>
Co-authored-by: Alex Huang <[email protected]>
Co-authored-by: Andrew Lamb <[email protected]>
comphead pushed a commit to comphead/arrow-datafusion that referenced this pull request Jul 2, 2024
…pache#11060)

* wip create and register ext file types with session

* Add contains function, and support in datafusion substrait consumer (apache#10879)

* adding new function contains

* adding substrait test

* adding doc

* adding doc

* Update docs/source/user-guide/sql/scalar_functions.md

Co-authored-by: Alex Huang <[email protected]>

* adding entry

---------

Co-authored-by: Alex Huang <[email protected]>

* logical planning updated

* compiling

* removing filetype enum

* compiling

* working on tests

* fix some tests

* test fixes

* cli fix

* cli fmt

* Update datafusion/core/src/datasource/file_format/mod.rs

Co-authored-by: Andrew Lamb <[email protected]>

* Update datafusion/core/src/execution/session_state.rs

Co-authored-by: Andrew Lamb <[email protected]>

* review comments

* review comments

* review comments

* typo fix

* fmt

* fix err log style

* fmt

---------

Co-authored-by: Lordworms <[email protected]>
Co-authored-by: Alex Huang <[email protected]>
Co-authored-by: Andrew Lamb <[email protected]>
findepi pushed a commit to findepi/datafusion that referenced this pull request Jul 16, 2024
…pache#10879)

* adding new function contains

* adding substrait test

* adding doc

* adding doc

* Update docs/source/user-guide/sql/scalar_functions.md

Co-authored-by: Alex Huang <[email protected]>

* adding entry

---------

Co-authored-by: Alex Huang <[email protected]>
findepi pushed a commit to findepi/datafusion that referenced this pull request Jul 16, 2024
…pache#11060)

* wip create and register ext file types with session

* Add contains function, and support in datafusion substrait consumer (apache#10879)

* adding new function contains

* adding substrait test

* adding doc

* adding doc

* Update docs/source/user-guide/sql/scalar_functions.md

Co-authored-by: Alex Huang <[email protected]>

* adding entry

---------

Co-authored-by: Alex Huang <[email protected]>

* logical planning updated

* compiling

* removing filetype enum

* compiling

* working on tests

* fix some tests

* test fixes

* cli fix

* cli fmt

* Update datafusion/core/src/datasource/file_format/mod.rs

Co-authored-by: Andrew Lamb <[email protected]>

* Update datafusion/core/src/execution/session_state.rs

Co-authored-by: Andrew Lamb <[email protected]>

* review comments

* review comments

* review comments

* typo fix

* fmt

* fix err log style

* fmt

---------

Co-authored-by: Lordworms <[email protected]>
Co-authored-by: Alex Huang <[email protected]>
Co-authored-by: Andrew Lamb <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sqllogictest SQL Logic Tests (.slt) substrait
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for contains function in datafusion substrait consumer
4 participants