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

Fully support LIKE/ILIKE with Utf8View #14379

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jan 30, 2025

Which issue does this PR close?

Rationale for this change

I wanted to ensure that we can fully support LIKE / ILIKE with UTF8View

It works today except for if the pattern was Utf8View but that turns out to be an over eager error in the planner -- the kernels work just fine

What changes are included in this PR?

  1. Update tests to show LIKE/ ILIKE work with Utf8View (and LargeUtf8 and Dictionary)
  2. Remove check in planner

Are these changes tested?

Yes

Are there any user-facing changes?

You can now run ... LIKE <Utf8View> column

@github-actions github-actions bot added sql SQL Planner sqllogictest SQL Logic Tests (.slt) labels Jan 30, 2025
@@ -819,10 +820,6 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
return not_impl_err!("ANY in LIKE expression");
}
let pattern = self.sql_expr_to_logical_expr(pattern, schema, planner_context)?;
let pattern_type = pattern.get_type(schema)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the code change -- don't do type checking in the planner

@@ -1689,6 +1689,15 @@ true true false false true true
statement ok
drop table t1

# can't use like with non stirngs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this shows what happens when you try to run like on an integer 😆

@@ -41,141 +41,6 @@ select arrow_cast(col1, 'Utf8') as c1 from test_substr_base;
#
include ./string_query.slt.part

# TODO support all String types in sql_like_to_expr and move this test to `string_query.slt.part`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By moving these tests to string_query.slt.part they now run for strings, large strings, stringview and dictionary arrays

FYI @goldmedal

@alamb alamb changed the title Fully support LIKE/NLIKE with Utf8View Fully support LIKE/ILIKE with Utf8View Jan 30, 2025
@alamb alamb marked this pull request as draft January 30, 2025 22:32
@alamb alamb marked this pull request as ready for review January 31, 2025 16:39
Copy link
Contributor

@2010YOUY01 2010YOUY01 left a comment

Choose a reason for hiding this comment

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

The change makes sense to me, thank you

I have one suggestion for additional test: now tests for StringView arrays are included, there are also tests for StringView literals can be added in https://github.com/apache/datafusion/blob/main/datafusion/sqllogictest/test_files/string/string_literal.slt, like select 'foo' like arrow_cast('foo', 'Utf8View')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sql SQL Planner sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants