Casting generic binary to generic string #3607
Conversation
arrow-cast/src/cast.rs
Outdated
| .iter() | ||
| .map(|maybe_value| match maybe_value { | ||
| Some(value) => { | ||
| let result = std::str::from_utf8(value); |
There was a problem hiding this comment.
The performance of this will be pretty poor, it would be better to cast the offsets and then use the optimised routine for UTF-8 validation
There was a problem hiding this comment.
This will be significantly faster than the approach here as it doesn't copy any string data and performs UTF-8 validation in a single pass (not to mention less code)
This can only be applied if CastOptions.safe as false. If CastOptions.safe as true, we still need to iterate and validate each value (because it will be null instead returning Err directly).
There was a problem hiding this comment.
Yeah, I keep forgetting about that option, it seems to be some thing we keep for legacy compatibility
viirya
force-pushed
the
binary_to_string
branch
from
23acc63
to
39aacbe
Compare
viirya
force-pushed
the
binary_to_string
branch
from
39aacbe
to
124205b
Compare
| let offsets = array.value_offsets(); | ||
| let values = array.value_data(); | ||
|
|
||
| // We only need to validate that all values are valid UTF-8 |
There was a problem hiding this comment.
It seems a shame to duplicate this logic, but I guess it is hard to avoid whilst having this type signature
|
Benchmark runs are scheduled for baseline = eeecbe5 and contender = 8cc8327. 8cc8327 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #3606.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?