LorrensP-2158466 commented on code in PR #11084:
URL: https://github.com/apache/datafusion/pull/11084#discussion_r1650527162
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -5494,6 +5566,69 @@ mod tests {
Ok(())
}
+ #[test]
+ #[allow(arithmetic_overflow)] // we want to test them
+ fn test_scalar_negative_overflows() -> Result<()> {
+ macro_rules! test_overflow_on_value {
+ ($($val:expr),* $(,)?) => {$(
+ {
+ let value: ScalarValue = $val;
+ let err = value.arithmetic_negate().expect_err("Should
receive overflow error on negating {value:?}");
+ let root_err = err.find_root();
+ match root_err{
+ DataFusionError::ArrowError(
+ ArrowError::ComputeError(_),
+ _,
+ ) => {}
+ _ => return Err(err),
+ };
+ }
+ )*};
+ }
+ test_overflow_on_value!(
+ // the integers
+ i8::MIN.into(),
+ i16::MIN.into(),
+ i32::MIN.into(),
+ i64::MIN.into(),
+ // for decimals, only value needs to be tested
+ ScalarValue::try_new_decimal128(i128::MIN, 10, 5)?,
+ ScalarValue::Decimal256(Some(i256::MIN), 20, 5),
+ // interval, check all possible values
+ ScalarValue::IntervalYearMonth(Some(i32::MIN)),
+ ScalarValue::new_interval_dt(i32::MIN, 999),
+ ScalarValue::new_interval_dt(1, i32::MIN),
+ ScalarValue::new_interval_mdn(i32::MIN, 15, 123_456),
+ ScalarValue::new_interval_mdn(12, i32::MIN, 123_456),
+ ScalarValue::new_interval_mdn(12, 15, i64::MIN),
+ // tz doesn't matter when negating
+ ScalarValue::TimestampSecond(Some(i64::MIN), None),
+ ScalarValue::TimestampMillisecond(Some(i64::MIN), None),
+ ScalarValue::TimestampMicrosecond(Some(i64::MIN), None),
+ ScalarValue::TimestampNanosecond(Some(i64::MIN), None),
+ );
+
+ let float_cases = [
+ (
+ ScalarValue::Float16(Some(f16::MIN)),
+ ScalarValue::Float16(Some(f16::MAX)),
+ ),
+ (
+ ScalarValue::Float16(Some(f16::MAX)),
+ ScalarValue::Float16(Some(f16::MIN)),
+ ),
+ (f32::MIN.into(), f32::MAX.into()),
+ (f32::MAX.into(), f32::MIN.into()),
+ (f64::MIN.into(), f64::MAX.into()),
+ (f64::MAX.into(), f64::MIN.into()),
+ ];
+ // skip float 16 because they aren't supported
+ for (test, expected) in float_cases.into_iter().skip(2) {
Review Comment:
Yeah I did this when filing #11083. Maybe it's cleaner to add a test that
should fail on Float16 and when Float16 is supported, that test fails and the
person making those changes knows to change the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
