diff --git a/native/spark-expr/src/math_funcs/abs.rs b/native/spark-expr/src/math_funcs/abs.rs index 5a16398ec4..7b79a15727 100644 --- a/native/spark-expr/src/math_funcs/abs.rs +++ b/native/spark-expr/src/math_funcs/abs.rs @@ -38,6 +38,7 @@ macro_rules! legacy_compute_op { }}; } +/// `$FROM_TYPE` is the Spark SQL type name carried in the `ArithmeticOverflow` payload. macro_rules! ansi_compute_op { ($ARRAY:expr, $FUNC:ident, $TYPE:ident, $RESULT:ident, $NATIVE:ident, $FROM_TYPE:expr) => {{ let n = $ARRAY.as_any().downcast_ref::<$TYPE>(); @@ -95,7 +96,7 @@ pub fn abs(args: &[ColumnarValue]) -> Result { let result = legacy_compute_op!(array, wrapping_abs, Int8Array, Int8Array); Ok(ColumnarValue::Array(Arc::new(result?))) } else { - ansi_compute_op!(array, abs, Int8Array, Int8Type, i8, "Int8") + ansi_compute_op!(array, abs, Int8Array, Int8Type, i8, "byte") } } DataType::Int16 => { @@ -103,7 +104,7 @@ pub fn abs(args: &[ColumnarValue]) -> Result { let result = legacy_compute_op!(array, wrapping_abs, Int16Array, Int16Array); Ok(ColumnarValue::Array(Arc::new(result?))) } else { - ansi_compute_op!(array, abs, Int16Array, Int16Type, i16, "Int16") + ansi_compute_op!(array, abs, Int16Array, Int16Type, i16, "short") } } DataType::Int32 => { @@ -111,7 +112,7 @@ pub fn abs(args: &[ColumnarValue]) -> Result { let result = legacy_compute_op!(array, wrapping_abs, Int32Array, Int32Array); Ok(ColumnarValue::Array(Arc::new(result?))) } else { - ansi_compute_op!(array, abs, Int32Array, Int32Type, i32, "Int32") + ansi_compute_op!(array, abs, Int32Array, Int32Type, i32, "integer") } } DataType::Int64 => { @@ -119,7 +120,7 @@ pub fn abs(args: &[ColumnarValue]) -> Result { let result = legacy_compute_op!(array, wrapping_abs, Int64Array, Int64Array); Ok(ColumnarValue::Array(Arc::new(result?))) } else { - ansi_compute_op!(array, abs, Int64Array, Int64Type, i64, "Int64") + ansi_compute_op!(array, abs, Int64Array, Int64Type, i64, "long") } } DataType::Float32 => { @@ -207,7 +208,7 @@ pub fn abs(args: &[ColumnarValue]) -> Result { // return the original value Ok(ColumnarValue::Scalar(ScalarValue::Int8(Some(*v)))) } else { - Err(arithmetic_overflow_error("Int8").into()) + Err(arithmetic_overflow_error("byte").into()) } } }, @@ -221,7 +222,7 @@ pub fn abs(args: &[ColumnarValue]) -> Result { // return the original value Ok(ColumnarValue::Scalar(ScalarValue::Int16(Some(*v)))) } else { - Err(arithmetic_overflow_error("Int16").into()) + Err(arithmetic_overflow_error("short").into()) } } }, @@ -235,7 +236,7 @@ pub fn abs(args: &[ColumnarValue]) -> Result { // return the original value Ok(ColumnarValue::Scalar(ScalarValue::Int32(Some(*v)))) } else { - Err(arithmetic_overflow_error("Int32").into()) + Err(arithmetic_overflow_error("integer").into()) } } }, @@ -249,7 +250,7 @@ pub fn abs(args: &[ColumnarValue]) -> Result { // return the original value Ok(ColumnarValue::Scalar(ScalarValue::Int64(Some(*v)))) } else { - Err(arithmetic_overflow_error("Int64").into()) + Err(arithmetic_overflow_error("long").into()) } } }, @@ -312,6 +313,7 @@ pub fn abs(args: &[ColumnarValue]) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::SparkError; use datafusion::common::cast::{ as_decimal128_array, as_decimal256_array, as_float32_array, as_float64_array, as_int16_array, as_int32_array, as_int64_array, as_int8_array, as_uint64_array, @@ -323,6 +325,87 @@ mod tests { } } + fn assert_spark_overflow(err: DataFusionError, expected_from_type: &str) { + if let DataFusionError::External(ref e) = err { + if let Some(SparkError::ArithmeticOverflow { from_type }) = + e.downcast_ref::() + { + assert_eq!(from_type, expected_from_type); + return; + } + } + panic!( + "Expected SparkError::ArithmeticOverflow {{ from_type: {expected_from_type:?} }}, got: {err:?}" + ); + } + + fn abs_ansi(value: ColumnarValue) -> Result { + abs(&[ + value, + ColumnarValue::Scalar(ScalarValue::Boolean(Some(true))), + ]) + } + + /// `abs(MIN)` under ANSI must name the Spark type, so the shims render `"long overflow"` + /// rather than `"Int64 overflow"`. + #[test] + fn test_ansi_abs_min_uses_spark_type_names() { + let arrays: Vec<(ArrayRef, &str)> = vec![ + (Arc::new(Int8Array::from(vec![i8::MIN])), "byte"), + (Arc::new(Int16Array::from(vec![i16::MIN])), "short"), + (Arc::new(Int32Array::from(vec![i32::MIN])), "integer"), + (Arc::new(Int64Array::from(vec![i64::MIN])), "long"), + ]; + for (array, from_type) in arrays { + assert_spark_overflow( + abs_ansi(ColumnarValue::Array(array)).unwrap_err(), + from_type, + ); + } + + for (scalar, from_type) in [ + (ScalarValue::Int8(Some(i8::MIN)), "byte"), + (ScalarValue::Int16(Some(i16::MIN)), "short"), + (ScalarValue::Int32(Some(i32::MIN)), "integer"), + (ScalarValue::Int64(Some(i64::MIN)), "long"), + ] { + assert_spark_overflow( + abs_ansi(ColumnarValue::Scalar(scalar)).unwrap_err(), + from_type, + ); + } + } + + /// The nearest valid input to each overflow boundary still succeeds, so the guard above is + /// not over-broad. + #[test] + fn test_ansi_abs_just_inside_boundary_succeeds() { + for (scalar, expected) in [ + ( + ScalarValue::Int8(Some(i8::MIN + 1)), + ScalarValue::Int8(Some(i8::MAX)), + ), + ( + ScalarValue::Int16(Some(i16::MIN + 1)), + ScalarValue::Int16(Some(i16::MAX)), + ), + ( + ScalarValue::Int32(Some(i32::MIN + 1)), + ScalarValue::Int32(Some(i32::MAX)), + ), + ( + ScalarValue::Int64(Some(i64::MIN + 1)), + ScalarValue::Int64(Some(i64::MAX)), + ), + ] { + let ColumnarValue::Scalar(result) = abs_ansi(ColumnarValue::Scalar(scalar)).unwrap() + else { + panic!("expected scalar result") + }; + assert_eq!(result, expected); + } + } + // Unsigned types, return as is #[test] fn test_abs_u8_scalar() { diff --git a/spark/src/test/resources/sql-tests/expressions/math/abs_ansi.sql b/spark/src/test/resources/sql-tests/expressions/math/abs_ansi.sql index c89a2958c1..879b24f3f6 100644 --- a/spark/src/test/resources/sql-tests/expressions/math/abs_ansi.sql +++ b/spark/src/test/resources/sql-tests/expressions/math/abs_ansi.sql @@ -18,6 +18,11 @@ -- ANSI mode abs function tests -- Tests that abs throws exceptions for overflow on minimum integer values +-- Spark 4.2 drops the type name from the abs overflow message and reports a bare +-- "[ARITHMETIC_OVERFLOW] overflow.", so the int and long patterns below cannot hold there. +-- abs_ansi_spark42.sql covers 4.2 and later with the loose pattern. +-- MaxSparkVersion: 4.1 + -- Config: spark.sql.ansi.enabled=true -- ============================================================================ @@ -53,11 +58,11 @@ INSERT INTO ansi_test_abs_byte VALUES (-128) -- ============================================================================ -- abs(-2147483648) cannot be represented as int (since INT_MAX = 2147483647) -query expect_error(overflow) +query expect_error(integer overflow) SELECT abs(v) FROM ansi_test_abs_int -- literal -query expect_error(overflow) +query expect_error(integer overflow) SELECT abs(-2147483648) -- ============================================================================ @@ -65,17 +70,21 @@ SELECT abs(-2147483648) -- ============================================================================ -- abs(-9223372036854775808) cannot be represented as long -query expect_error(overflow) +query expect_error(long overflow) SELECT abs(v) FROM ansi_test_abs_long -- literal -query expect_error(overflow) +query expect_error(long overflow) SELECT abs(-9223372036854775808L) -- ============================================================================ -- abs(SHORT_MIN) overflow -- ============================================================================ +-- Byte and short stay on the loose `overflow` pattern. Spark 4.x reports +-- "byte overflow" / "short overflow", but 3.4 and 3.5 raise +-- _LEGACY_ERROR_TEMP_2043 ("- caused overflow.") instead, and +-- expect_error asserts the pattern against Spark's message as well as Comet's. -- abs(-32768) cannot be represented as short query expect_error(overflow) SELECT abs(v) FROM ansi_test_abs_short diff --git a/spark/src/test/resources/sql-tests/expressions/math/abs_ansi_spark42.sql b/spark/src/test/resources/sql-tests/expressions/math/abs_ansi_spark42.sql new file mode 100644 index 0000000000..6e8ec50094 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/math/abs_ansi_spark42.sql @@ -0,0 +1,103 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. + +-- ANSI mode abs function tests +-- Tests that abs throws exceptions for overflow on minimum integer values + +-- Spark 4.2 reports a bare "[ARITHMETIC_OVERFLOW] overflow." for abs, with no type name, so every +-- pattern here is the loose "overflow". abs_ansi.sql covers 4.1 and earlier, where int and long +-- carry the type name and are asserted exactly. Comet still emits the Spark type names on every +-- version, which the Rust tests in abs.rs assert directly. +-- MinSparkVersion: 4.2 + +-- Config: spark.sql.ansi.enabled=true + +-- ============================================================================ +-- Test data setup +-- ============================================================================ + +statement +CREATE TABLE ansi_test_abs_int(v int) USING parquet + +statement +INSERT INTO ansi_test_abs_int VALUES (-2147483648) + +statement +CREATE TABLE ansi_test_abs_long(v long) USING parquet + +statement +INSERT INTO ansi_test_abs_long VALUES (-9223372036854775808) + +statement +CREATE TABLE ansi_test_abs_short(v short) USING parquet + +statement +INSERT INTO ansi_test_abs_short VALUES (-32768) + +statement +CREATE TABLE ansi_test_abs_byte(v tinyint) USING parquet + +statement +INSERT INTO ansi_test_abs_byte VALUES (-128) + +-- ============================================================================ +-- abs(INT_MIN) overflow +-- ============================================================================ + +-- abs(-2147483648) cannot be represented as int (since INT_MAX = 2147483647) +query expect_error(overflow) +SELECT abs(v) FROM ansi_test_abs_int + +-- literal +query expect_error(overflow) +SELECT abs(-2147483648) + +-- ============================================================================ +-- abs(LONG_MIN) overflow +-- ============================================================================ + +-- abs(-9223372036854775808) cannot be represented as long +query expect_error(overflow) +SELECT abs(v) FROM ansi_test_abs_long + +-- literal +query expect_error(overflow) +SELECT abs(-9223372036854775808L) + +-- ============================================================================ +-- abs(SHORT_MIN) overflow +-- ============================================================================ + +-- abs(-32768) cannot be represented as short +query expect_error(overflow) +SELECT abs(v) FROM ansi_test_abs_short + +-- literal +query expect_error(overflow) +SELECT abs(cast(-32768 as short)) + +-- ============================================================================ +-- abs(BYTE_MIN) overflow +-- ============================================================================ + +-- abs(-128) cannot be represented as tinyint +query expect_error(overflow) +SELECT abs(v) FROM ansi_test_abs_byte + +-- literal +query expect_error(overflow) +SELECT abs(cast(-128 as tinyint))