Add Datum Arithmetic tests, Fix Interval Substraction (#4480)#4493
Add Datum Arithmetic tests, Fix Interval Substraction (#4480)#4493tustvold merged 4 commits intoapache:masterfrom
Conversation
| assert_eq!( | ||
| c.value(0), | ||
| Date32Type::from_naive_date(NaiveDate::from_ymd_opt(2023, 3, 27).unwrap()) | ||
| Date32Type::from_naive_date(NaiveDate::from_ymd_opt(2023, 3, 28).unwrap()) |
There was a problem hiding this comment.
The previous logic was incorrect but happened to give a potentially plausible result. I'm not sure what the correct rounding behaviour is here though
There was a problem hiding this comment.
86500 is milliseconds so 86.5 seconds before 2023-03-29
Thus 2023-03-28 seems much more correct to me
tustvold
changed the title
| assert_eq!( | ||
| c.value(0), | ||
| Date32Type::from_naive_date(NaiveDate::from_ymd_opt(2023, 3, 27).unwrap()) | ||
| Date32Type::from_naive_date(NaiveDate::from_ymd_opt(2023, 3, 28).unwrap()) |
There was a problem hiding this comment.
86500 is milliseconds so 86.5 seconds before 2023-03-29
Thus 2023-03-28 seems much more correct to me
| } | ||
|
|
||
| impl std::fmt::Display for Op { | ||
| fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { |
|
|
||
| let l_mul = T::Native::usize_as(10).pow_wrapping((result_scale - s1) as _); | ||
| let r_mul = T::Native::usize_as(10).pow_wrapping((result_scale - s2) as _); | ||
| let l_mul = T::Native::usize_as(10).pow_checked((result_scale - s1) as _)?; |
There was a problem hiding this comment.
I was a little confused at first about why both AddWrapping and Add using the same function. Then
I realized this is for computing the scale factor
Given MAX_PRECISION is a constant, what is the reason to change this to pow_checked? Maybe it doesn't matter performance wise, I was just curious
There was a problem hiding this comment.
It can overflow on negative scales
| let a = UInt8Array::from(vec![56, 5, 3]); | ||
| let b = UInt8Array::from(vec![200, 2, 5]); | ||
| let err = add(&a, &b).unwrap_err().to_string(); | ||
| assert_eq!(err, "Compute error: Overflow happened on: 56 + 200"); |
| let a = Int16Array::from(vec![21]); | ||
| let b = Int16Array::from(vec![0]); | ||
| let err = div(&a, &b).unwrap_err().to_string(); | ||
| assert_eq!(err, "Divide by zero error"); |
There was a problem hiding this comment.
I didn't see a test for integer remainder %
| .with_precision_and_scale(37, 37) | ||
| .unwrap(); | ||
| let err = mul(&a, &b).unwrap_err().to_string(); | ||
| assert_eq!(err, "Invalid argument error: Output scale of Decimal128(3, 3) * Decimal128(37, 37) would exceed max scale of 38"); |
| let a = PrimitiveArray::<T>::new(values, None); | ||
| let b = IntervalYearMonthArray::from(vec
Which issue does this PR close?
Closes #4480
Closes #4489
Closes #1065
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?