Cleanup cast kernel #2846
Cleanup cast kernel #2846
Conversation
| @@ -1617,33 +1561,6 @@ fn cast_decimal_to_decimal<const BYTE_WIDTH1: usize, const BYTE_WIDTH2: usize>( | |||
| } | |||
| } | |||
|
|
|||
| /// Cast an array by changing its array_data type to the desired type | |||
There was a problem hiding this comment.
This function isn't sound, as it basically allows arbitrary transmutation of ArrayData 😅
Tbh this was the major motivator for this PR, whilst unsoundness in internal APIs is not nearly as bad as in public APIs, it is still something to be avoided.
|
Need to revisit how this is handling timestamps |
| @@ -769,11 +769,16 @@ impl<T: ArrowTimestampType> PrimitiveArray<T> { | |||
|
|
|||
| /// Construct a timestamp array with new timezone | |||
| pub fn with_timezone(&self, timezone: String) -> Self { | |||
| self.with_timezone_opt(Some(timezone)) | |||
| Ok(make_timestamp_array( | ||
| &converted, | ||
| to_unit.clone(), | ||
| to_tz.clone(), | ||
| )) |
There was a problem hiding this comment.
Hmm, for this kind of conversion between timezones, I think from_tz must be non empty.
There was a problem hiding this comment.
Not sure, the values should be UTC. Regardless this was the prior behaviour and there is a ticket already taking this #1936
There was a problem hiding this comment.
Okay as it is the prior behavior.
|
As ever, thank you for reviewing my PRs 😄 |
|
Benchmark runs are scheduled for baseline = 5cf46d4 and contender = 9db7518. 9db7518 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
|
@tustvold You're welcome! ❤️ Thank you for reviewing my PRs too 😄 |
Which issue does this PR close?
Closes #.
Rationale for this change
The cast kernel predates a lot of the work to introduce safe APIs for interacting with arrays, inspired by the work of arrow2. This updates the cast kernel to make use of these APIs 🎉
What changes are included in this PR?
Uses PrimitiveArray::reinterpret_cast added by #2785 to reduce use of unsafe
Cleans up some other callsites
Are there any user-facing changes?