[arrow-cast] Support cast numeric to string view #6719
Conversation
Signed-off-by: Tai Le Manh <[email protected]>
|
@Omega359 During the code review, I found that it is possible to implement support cast for all numeric values ( |
tlm365
force-pushed
the
cast_numeric_to_string_view
branch
from
f4dfcda
to
2a937fe
Compare
arrow-cast/src/cast/string.rs
Outdated
| let nulls = array.nulls(); | ||
| for i in 0..array.len() { | ||
| match nulls.map(|x| x.is_null(i)).unwrap_or_default() { | ||
| false => builder.append_value(formatter.value(i).try_to_string()?), |
There was a problem hiding this comment.
It would be more optimal to use the std::fmt::write support as for StringArray above.
As written this will allocate for every value which will be very expensive
There was a problem hiding this comment.
@tustvold Thanks so much for reviewing.
It would be more optimal to use the std::fmt::write support as for StringArray above.
As written this will allocate for every value which will be very expensive
I get it now. Will try to implement it. TYSM ❤️
Signed-off-by: Tai Le Manh <[email protected]>
arrow-cast/src/cast/mod.rs
Outdated
| @@ -1462,6 +1464,9 @@ pub fn cast_with_options( | |||
| (BinaryView, _) => Err(ArrowError::CastError(format!( | |||
| "Casting from {from_type:?} to {to_type:?} not supported", | |||
| ))), | |||
| (from_type, Utf8View) if from_type.is_primitive() => { | |||
There was a problem hiding this comment.
I believe this also fixes the Timestamp -> Utf8View issue. It would be good to have tests for temporal -> Utf8View added to cover this case.
There was a problem hiding this comment.
After reviewing the code, I realized that the Timestamp -> Utf8View cast is not supported yet.
The main issue comes from the current implementation of formatter.format.write (source) which currently only applies to DisplayIndex derives (source), but the Temporal datatype is implemented based on DisplayIndexState (source).
I think this issue deserves a separate PR to handle the temporal -> string view casting.
There was a problem hiding this comment.
I'll file another PR today to cover the temporal -> Utf8View case unless someone beats me to it.
Signed-off-by: Tai Le Manh <[email protected]>
tlm365
force-pushed
the
cast_numeric_to_string_view
branch
from
1a6868a
to
74de9bc
Compare
Which issue does this PR close?
Closes #6714.
Rationale for this change
Add support cast from numeric(
Int/Float/Decimal) to string view (Utf8View).What changes are included in this PR?
The cast logic and corresponding unit tests.
Are there any user-facing changes?
No.