diff --git a/native/spark-expr/benches/cast_from_boolean.rs b/native/spark-expr/benches/cast_from_boolean.rs index 04bd72dc01..caccd67e26 100644 --- a/native/spark-expr/benches/cast_from_boolean.rs +++ b/native/spark-expr/benches/cast_from_boolean.rs @@ -69,20 +69,7 @@ fn criterion_benchmark(c: &mut Criterion) { None, None, ); - let cast_to_str = Cast::new( - expr.clone(), - DataType::Utf8, - spark_cast_options.clone(), - None, - None, - ); - let cast_to_decimal = Cast::new( - expr, - DataType::Decimal128(10, 4), - spark_cast_options, - None, - None, - ); + let cast_to_str = Cast::new(expr, DataType::Utf8, spark_cast_options, None, None); let mut group = c.benchmark_group("cast_bool".to_string()); group.bench_function("i8", |b| { @@ -106,9 +93,6 @@ fn criterion_benchmark(c: &mut Criterion) { group.bench_function("str", |b| { b.iter(|| cast_to_str.evaluate(&boolean_batch).unwrap()); }); - group.bench_function("decimal", |b| { - b.iter(|| cast_to_decimal.evaluate(&boolean_batch).unwrap()); - }); } fn create_boolean_batch() -> RecordBatch { diff --git a/native/spark-expr/src/conversion_funcs/boolean.rs b/native/spark-expr/src/conversion_funcs/boolean.rs index 3f4a86429c..1db2746ce2 100644 --- a/native/spark-expr/src/conversion_funcs/boolean.rs +++ b/native/spark-expr/src/conversion_funcs/boolean.rs @@ -15,11 +15,13 @@ // specific language governing permissions and limitations // under the License. -use crate::{SparkError, SparkResult}; -use arrow::array::{Array, ArrayRef, AsArray, Decimal128Array, TimestampMicrosecondBuilder}; +use crate::SparkResult; +use arrow::array::{Array, ArrayRef, AsArray, TimestampMicrosecondBuilder}; use arrow::datatypes::DataType; use std::sync::Arc; +/// Boolean -> Decimal is intentionally absent: it has no native path and is routed through the +/// JVM codegen dispatcher on the Scala side (see `CometCast.canCastFromBoolean`). pub fn is_df_cast_from_bool_spark_compatible(to_type: &DataType) -> bool { use DataType::*; matches!( @@ -28,35 +30,6 @@ pub fn is_df_cast_from_bool_spark_compatible(to_type: &DataType) -> bool { ) } -pub fn cast_boolean_to_decimal( - array: &ArrayRef, - precision: u8, - scale: i8, -) -> SparkResult { - let bool_array = array.as_boolean(); - let scaled_val = 10_i128.pow(scale as u32); - let result: Decimal128Array = bool_array - .iter() - .map(|v| v.map(|b| if b { scaled_val } else { 0 })) - .collect(); - - // Convert Arrow decimal overflow errors to SparkError - let decimal_array = result - .with_precision_and_scale(precision, scale) - .map_err(|e| { - if matches!(e, arrow::error::ArrowError::InvalidArgumentError(_)) - && e.to_string().contains("too large to store in a Decimal128") - { - // Use the scaled value as it's the only non-zero value that could overflow - crate::error::decimal_overflow_error(scaled_val, precision, scale) - } else { - SparkError::Arrow(Arc::new(e)) - } - })?; - - Ok(Arc::new(decimal_array)) -} - pub(crate) fn cast_boolean_to_timestamp( array_ref: &ArrayRef, target_tz: &Option>, @@ -212,20 +185,20 @@ mod tests { } #[test] - fn test_bool_to_decimal_cast() { - let result = cast_array( + fn test_bool_to_decimal_cast_is_not_supported() { + // Boolean -> Decimal has no native path; the Scala planner routes it through the JVM + // codegen dispatcher, so reaching the native cast at all is a bug. + let err = cast_array( test_input_bool_array(), &Decimal128(10, 4), &test_input_spark_opts(), ) - .unwrap(); - let expected_arr = Decimal128Array::from(vec![10000_i128, 0_i128]) - .with_precision_and_scale(10, 4) - .unwrap(); - let arr = result.as_any().downcast_ref::().unwrap(); - assert_eq!(arr.value(0), expected_arr.value(0)); - assert_eq!(arr.value(1), expected_arr.value(1)); - assert!(arr.is_null(2)); + .expect_err("expected boolean -> decimal to be unsupported"); + assert!( + err.to_string() + .contains("Native cast invoked for unsupported cast"), + "unexpected error: {err}" + ); } #[test] diff --git a/native/spark-expr/src/conversion_funcs/cast.rs b/native/spark-expr/src/conversion_funcs/cast.rs index 37fddb8c11..504ea6695e 100644 --- a/native/spark-expr/src/conversion_funcs/cast.rs +++ b/native/spark-expr/src/conversion_funcs/cast.rs @@ -16,7 +16,7 @@ // under the License. use crate::conversion_funcs::boolean::{ - cast_boolean_to_decimal, cast_boolean_to_timestamp, is_df_cast_from_bool_spark_compatible, + cast_boolean_to_timestamp, is_df_cast_from_bool_spark_compatible, }; use crate::conversion_funcs::numeric::{ cast_decimal128_to_utf8, cast_decimal_to_timestamp, cast_float32_to_decimal128, @@ -438,9 +438,6 @@ pub(crate) fn cast_array( (Int64, Binary) if (eval_mode == Legacy) => { cast_whole_num_to_binary!(&array, Int64Array, 8) } - (Boolean, Decimal128(precision, scale)) => { - cast_boolean_to_decimal(&array, *precision, *scale) - } (Int8 | Int16 | Int32 | Int64, Timestamp(_, tz)) => cast_int_to_timestamp(&array, tz), (Float32 | Float64, Timestamp(_, tz)) => cast_float_to_timestamp(&array, tz, eval_mode), (Boolean, Timestamp(_, tz)) => cast_boolean_to_timestamp(&array, tz), diff --git a/spark/src/main/scala/org/apache/comet/expressions/CometCast.scala b/spark/src/main/scala/org/apache/comet/expressions/CometCast.scala index eba1e4b152..cadaa48046 100644 --- a/spark/src/main/scala/org/apache/comet/expressions/CometCast.scala +++ b/spark/src/main/scala/org/apache/comet/expressions/CometCast.scala @@ -347,10 +347,14 @@ object CometCast private def canCastFromBoolean(toType: DataType, evalMode: CometEvalMode.Value): SupportLevel = toType match { case DataTypes.ByteType | DataTypes.ShortType | DataTypes.IntegerType | DataTypes.LongType | - DataTypes.FloatType | DataTypes.DoubleType | _: DecimalType => + DataTypes.FloatType | DataTypes.DoubleType => Compatible() case _: TimestampType if evalMode == CometEvalMode.LEGACY => Compatible() + // Boolean -> Decimal has no native path. It is a rare cast and getting the + // precision/scale/overflow behavior right in native code is not worth the complexity, so + // the `CodegenDispatchFallback` mixin routes it through Spark's own generated code inside + // the Comet pipeline instead. case _ => unsupported(DataTypes.BooleanType, toType) } diff --git a/spark/src/test/resources/sql-tests/expressions/cast/cast_boolean_to_decimal.sql b/spark/src/test/resources/sql-tests/expressions/cast/cast_boolean_to_decimal.sql new file mode 100644 index 0000000000..c995fb26d8 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/cast/cast_boolean_to_decimal.sql @@ -0,0 +1,72 @@ +-- 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. + +-- Boolean -> Decimal has no native path in Comet. `CometCast` reports it as unsupported so the +-- `CodegenDispatchFallback` mixin routes it through Spark's own generated code inside the Comet +-- pipeline. The default `query` mode therefore still asserts fully native execution. + +statement +CREATE TABLE test_cast_bool_to_decimal(id int, b boolean) USING parquet + +statement +INSERT INTO test_cast_bool_to_decimal VALUES (1, true), (2, false), (3, NULL) + +-- basic precision/scale combinations, including the long fast path (precision <= 18) and the +-- BigDecimal path (precision > 18) +query +SELECT id, cast(b as decimal(10,2)), cast(b as decimal(14,4)), cast(b as decimal(30,0)) +FROM test_cast_bool_to_decimal ORDER BY id + +-- smallest precision/scale that can represent 1 +query +SELECT id, cast(b as decimal(1,0)), cast(b as decimal(2,1)) +FROM test_cast_bool_to_decimal ORDER BY id + +-- maximum precision, with and without scale +query +SELECT id, cast(b as decimal(38,0)), cast(b as decimal(38,37)) +FROM test_cast_bool_to_decimal ORDER BY id + +-- overflow: decimal(1,1) holds at most 0.9, so true does not fit and returns NULL in non-ANSI +-- mode while false and NULL are unaffected +query +SELECT id, cast(b as decimal(1,1)), cast(b as decimal(2,2)), cast(b as decimal(38,38)) +FROM test_cast_bool_to_decimal ORDER BY id + +-- literal arguments +query +SELECT cast(true as decimal(10,2)), cast(false as decimal(10,2)), + cast(cast(NULL as boolean) as decimal(10,2)) + +-- literal overflow +query +SELECT cast(true as decimal(1,1)), cast(false as decimal(1,1)) + +-- try_cast returns NULL on overflow rather than throwing +query +SELECT id, try_cast(b as decimal(10,2)), try_cast(b as decimal(1,1)) +FROM test_cast_bool_to_decimal ORDER BY id + +-- comparison predicates on the result of the cast +query +SELECT id FROM test_cast_bool_to_decimal +WHERE cast(b as decimal(10,2)) > 0.5 ORDER BY id + +-- the cast feeding an aggregate +query +SELECT sum(cast(b as decimal(10,2))), count(cast(b as decimal(1,1))) +FROM test_cast_bool_to_decimal diff --git a/spark/src/test/resources/sql-tests/expressions/cast/cast_boolean_to_decimal_ansi.sql b/spark/src/test/resources/sql-tests/expressions/cast/cast_boolean_to_decimal_ansi.sql new file mode 100644 index 0000000000..1025dbbee5 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/cast/cast_boolean_to_decimal_ansi.sql @@ -0,0 +1,79 @@ +-- 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 edge cases for Boolean -> Decimal. Comet has no native path for this cast; the +-- `CodegenDispatchFallback` mixin runs Spark's own generated code inside the Comet pipeline, so +-- the ANSI overflow errors must match Spark exactly. The non-error queries act as sentinels +-- proving the cast really executed natively rather than falling the whole plan back to Spark. + +-- Config: spark.sql.ansi.enabled=true + +statement +CREATE TABLE test_cast_bool_to_decimal_ansi(id int, b boolean) USING parquet + +statement +INSERT INTO test_cast_bool_to_decimal_ansi VALUES (1, true), (2, false), (3, NULL) + +-- sentinel: a cast that always fits must run natively under ANSI mode +query +SELECT id, cast(b as decimal(10,2)), cast(b as decimal(38,0)) +FROM test_cast_bool_to_decimal_ansi ORDER BY id + +-- sentinel: decimal(1,0) is the tightest type that can hold 1 +query +SELECT id, cast(b as decimal(1,0)), cast(b as decimal(2,1)) +FROM test_cast_bool_to_decimal_ansi ORDER BY id + +-- decimal(1,1) holds at most 0.9, so casting true overflows and must throw under ANSI mode +query expect_error(NUMERIC_VALUE_OUT_OF_RANGE) +SELECT cast(b as decimal(1,1)) FROM test_cast_bool_to_decimal_ansi WHERE id = 1 + +-- decimal(2,2) holds at most 0.99: same overflow, larger precision +query expect_error(NUMERIC_VALUE_OUT_OF_RANGE) +SELECT cast(b as decimal(2,2)) FROM test_cast_bool_to_decimal_ansi WHERE id = 1 + +-- all-scale decimal at maximum precision still cannot represent 1 +query expect_error(NUMERIC_VALUE_OUT_OF_RANGE) +SELECT cast(b as decimal(38,38)) FROM test_cast_bool_to_decimal_ansi WHERE id = 1 + +-- literal true overflows the same way +query expect_error(NUMERIC_VALUE_OUT_OF_RANGE) +SELECT cast(true as decimal(1,1)) + +-- overflow is value dependent: false scales to 0, which fits every decimal type, so rows that +-- only contain false must not throw +query +SELECT cast(b as decimal(1,1)), cast(b as decimal(38,38)) +FROM test_cast_bool_to_decimal_ansi WHERE id = 2 + +-- NULL input short-circuits before the precision check, so it must not throw either +query +SELECT cast(b as decimal(1,1)), cast(b as decimal(38,38)) +FROM test_cast_bool_to_decimal_ansi WHERE id = 3 + +-- try_cast suppresses the ANSI overflow and yields NULL +query +SELECT id, try_cast(b as decimal(1,1)), try_cast(b as decimal(38,38)) +FROM test_cast_bool_to_decimal_ansi ORDER BY id + +-- overflow raised from inside an aggregate input +query expect_error(NUMERIC_VALUE_OUT_OF_RANGE) +SELECT sum(cast(b as decimal(1,1))) FROM test_cast_bool_to_decimal_ansi + +-- overflow raised from inside a filter predicate +query expect_error(NUMERIC_VALUE_OUT_OF_RANGE) +SELECT id FROM test_cast_bool_to_decimal_ansi WHERE cast(b as decimal(1,1)) > 0.5 diff --git a/spark/src/test/scala/org/apache/comet/CometCastSuite.scala b/spark/src/test/scala/org/apache/comet/CometCastSuite.scala index fa5178cdfd..722535c22f 100644 --- a/spark/src/test/scala/org/apache/comet/CometCastSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometCastSuite.scala @@ -177,10 +177,25 @@ class CometCastSuite extends CometTestBase with AdaptiveSparkPlanHelper { castTest(generateBools(), DataTypes.DoubleType) } - test("cast BooleanType to DecimalType(10,2)") { + // Boolean -> Decimal has no native path and is routed through the JVM codegen dispatcher, so + // `CometCast.isSupported` reports it as unsupported and the matrix-consistency test above + // requires this one to be ignored. Execution coverage, including the ANSI overflow edge cases, + // lives in `sql-tests/expressions/cast/cast_boolean_to_decimal{,_ansi}.sql`. + ignore("cast BooleanType to DecimalType(10,2)") { castTest(generateBools(), DataTypes.createDecimalType(10, 2)) } + test("cast BooleanType to DecimalType is not supported natively") { + Seq( + DataTypes.createDecimalType(10, 2), + DataTypes.createDecimalType(14, 4), + DataTypes.createDecimalType(30, 0)).foreach { toType => + assert( + CometCast.isSupported(BooleanType, toType, None, CometEvalMode.LEGACY) == + Unsupported(Some(s"Cast from $BooleanType to $toType is not supported"))) + } + } + test("cast BooleanType to DecimalType(14,4)") { castTest(generateBools(), DataTypes.createDecimalType(14, 4)) }