Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 1 addition & 17 deletions native/spark-expr/benches/cast_from_boolean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand All @@ -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 {
Expand Down
55 changes: 14 additions & 41 deletions native/spark-expr/src/conversion_funcs/boolean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand All @@ -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<ArrayRef> {
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<Arc<str>>,
Expand Down Expand Up @@ -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::<Decimal128Array>().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]
Expand Down
5 changes: 1 addition & 4 deletions native/spark-expr/src/conversion_funcs/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
17 changes: 16 additions & 1 deletion spark/src/test/scala/org/apache/comet/CometCastSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this ignored test? are we planning to switch it back after future changes?

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))
}
Expand Down
Loading