Cast decimal256 to signed integer #3040
Conversation
| for i in 0..array.len() { | ||
| if array.is_null(i) { | ||
| value_builder.append_null(); | ||
| fn cast_decimal_to_integer<D, T>( |
There was a problem hiding this comment.
Do we need to support castoptions here?
There was a problem hiding this comment.
castoptions is not supported originally for casting from decimal128 to signed integer. We should support it actually. Can be in this PR or a later PR.
arrow-buffer/src/bigint.rs
Outdated
|
|
||
| impl AsPrimitive<$native_ty> for i256 { | ||
| fn as_(self) -> $native_ty { | ||
| let integer = self.to_i128(); |
There was a problem hiding this comment.
I think this should just be self.low as _?
arrow-cast/src/cast.rs
Outdated
| array: &ArrayRef, | ||
| base: D::Native, | ||
| scale: u8, | ||
| max_bound: D::Native, |
There was a problem hiding this comment.
We could implement ToPrimitive for i256 and then use NumCast here?
There was a problem hiding this comment.
Implemented ToPrimitive for i256 now.
| let (low_bytes, high_bytes) = split_array(u128::to_le_bytes(self.low)); | ||
| let high = i64::from_le_bytes(high_bytes); | ||
| let low = i64::from_le_bytes(low_bytes); | ||
|
|
||
| let high_negative = high < 0; | ||
| let low_negative = low < 0; | ||
| let high_valid = self.high == -1 || self.high == 0; | ||
|
|
||
| (high_negative == low_negative && high_valid).then_some(low) |
There was a problem hiding this comment.
I tried simply calling this.low.to_i64(), but it cannot pass
let a = i256::from_i128(i64::MIN as i128);
assert_eq!(a.to_i64().unwrap(), i64::MIN);
arrow-cast/src/cast.rs
Outdated
| let v = array.value(i).div_checked(div)?; | ||
|
|
||
| // check the overflow | ||
| // For example: Decimal(128,10,0) as i8 |
There was a problem hiding this comment.
Can we just use append_option(NumCast::from(v)) and eliminate the bounds checks from this code?
There was a problem hiding this comment.
Yea, but it requires CastOptions so it won't break existing tests. I added CastOptions together.
| @@ -478,6 +480,44 @@ define_as_primitive!(i16); | |||
| define_as_primitive!(i32); | |||
| define_as_primitive!(i64); | |||
|
|
|||
| impl ToPrimitive for i256 { | |||
There was a problem hiding this comment.
I think it might be possible to simplify this, I'll have a quick play this afternoon
viirya
force-pushed
the
cast_decimal256_to_integer
branch
from
15639b8 to
11f1528
Compare
|
Benchmark runs are scheduled for baseline = 879b461 and contender = a950b52. a950b52 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 #3039.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?