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

fix: dont try to coerce list for regex match #11646

Merged
merged 3 commits into from
Jul 26, 2024

Conversation

tshauck
Copy link
Contributor

@tshauck tshauck commented Jul 25, 2024

Which issue does this PR close?

Closes #11622

Rationale for this change

Regex match's args will try to be coerced from Lists, but that shouldn't be possible.

List coercion was added in #10445, but it's not clear to me why.

What changes are included in this PR?

Remove list coercion, add tests

Are these changes tested?

yes

Are there any user-facing changes?

No

@tshauck tshauck changed the title fix: dont try to coerce list for regex fix: dont try to coerce list for regex match Jul 25, 2024
@github-actions github-actions bot added the logical-expr Logical plan and expressions label Jul 25, 2024
@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Jul 25, 2024
@@ -325,6 +325,11 @@ SELECT 'foo\nbar\nbaz' ~ 'bar';
----
true

statement error
Error during planning: Cannot infer common argument type for regex operation List(Field { name: "item", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A bit verbose, but tries to prove the error happens during planning.

@tshauck tshauck marked this pull request as ready for review July 25, 2024 02:30
@jayzhan211
Copy link
Contributor

I guess we don't need list coercion for Like too

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.

👍

@alamb
Copy link
Contributor

alamb commented Jul 25, 2024

cc @b41sh (as the author of #10445) in case you have extra context you would like to share

@tshauck
Copy link
Contributor Author

tshauck commented Jul 25, 2024

I guess we don't need list coercion for Like too

+1, I'll wait a day or so to hear if it's there for a reason for it, otherwise I'll pull it out too.

@alamb alamb merged commit cdf387e into apache:main Jul 26, 2024
24 checks passed
@alamb
Copy link
Contributor

alamb commented Jul 26, 2024

Thanks again @tshauck and @jayzhan211

@tshauck tshauck deleted the fix-regex-coercion branch July 26, 2024 15:03
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 sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internal error when regex operator ~ is used with Lists (SQLancer)
3 participants