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

binary_to_string_coercion in binary.rs does not support utf8view #13361

Closed
Omega359 opened this issue Nov 11, 2024 · 3 comments · May be fixed by #13370
Closed

binary_to_string_coercion in binary.rs does not support utf8view #13361

Omega359 opened this issue Nov 11, 2024 · 3 comments · May be fixed by #13370
Labels
bug Something isn't working

Comments

@Omega359
Copy link
Contributor

Describe the bug

The coercion rules defined in binary_to_string_coercion do not account for utf8view

To Reproduce

No response

Expected behavior

binary coercion rules support utf8view

Additional context

No response

@alamb
Copy link
Contributor

alamb commented Jan 30, 2025

i double checked and binary_to_string_coercion already suports Utf8View so closing this issue

use arrow::datatypes::DataType::*;
match (lhs_type, rhs_type) {
(Binary, Utf8) => Some(Utf8),
(Binary, LargeUtf8) => Some(LargeUtf8),
(BinaryView, Utf8) => Some(Utf8View),
(BinaryView, LargeUtf8) => Some(LargeUtf8),
(LargeBinary, Utf8) => Some(LargeUtf8),
(LargeBinary, LargeUtf8) => Some(LargeUtf8),
(Utf8, Binary) => Some(Utf8),
(Utf8, LargeBinary) => Some(LargeUtf8),
(Utf8, BinaryView) => Some(Utf8View),
(LargeUtf8, Binary) => Some(LargeUtf8),
(LargeUtf8, LargeBinary) => Some(LargeUtf8),
(LargeUtf8, BinaryView) => Some(LargeUtf8),
_ => None,
}

@alamb
Copy link
Contributor

alamb commented Jan 30, 2025

I see now that @jonathanc-n proposes some additions in

However I am not sure how important those cases are to cover (how common they are in practice). So I propose closing this ticket as "good enough" unless someone hits them for real.

Funnily enough while verifying the tests I found one small corner case;

@alamb
Copy link
Contributor

alamb commented Feb 3, 2025

Let's reopen this ticket if we find any additional work to do -- I think it is closed enough in #14379

@alamb alamb closed this as completed Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants