-
Notifications
You must be signed in to change notification settings - Fork 850
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
Added casting from Binary/LargeBinary to Utf8View #6592
Added casting from Binary/LargeBinary to Utf8View #6592
Conversation
arrow-cast/src/cast/mod.rs
Outdated
Utf8View => { | ||
let array = cast_binary_to_string::<i64>(array, cast_options)?; | ||
Ok(Arc::new(StringViewArray::from( | ||
cast_byte_container::<LargeUtf8Type, Utf8Type>(array.as_ref())? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could result in offset overflow, is this cast required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played around with this and found that StringViewArray::from
works directly from a LargeStringArray
and pushed a commit I think helps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also made a PR to help document #6610
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arrow-cast/src/cast/mod.rs
Outdated
Utf8View => { | ||
let array = cast_binary_to_string::<i64>(array, cast_options)?; | ||
Ok(Arc::new(StringViewArray::from( | ||
cast_byte_container::<LargeUtf8Type, Utf8Type>(array.as_ref())? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played around with this and found that StringViewArray::from
works directly from a LargeStringArray
and pushed a commit I think helps.
assert!(can_cast_types( | ||
binary_array.data_type(), | ||
&DataType::BinaryView | ||
)); | ||
|
||
let string_view_array = cast(&binary_array, &DataType::Utf8View).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I double checked that this is invoked with both BinaryArray
and LargeBinaryArray
Which issue does this PR close?
Closes #6531.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?