This is an automated email from the ASF dual-hosted git repository.
andygrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new f22d9f17bb refactor: use arity helper for Int to Decimal128
reinterpretation (#5193)
f22d9f17bb is described below
commit f22d9f17bb99b5dd60768e8cafd4dd0e312756de
Author: ChenChen Lai <[email protected]>
AuthorDate: Thu Aug 6 02:02:15 2026 +0800
refactor: use arity helper for Int to Decimal128 reinterpretation (#5193)
---
native/core/src/execution/columnar_to_row.rs | 181 +++++++++++------
native/spark-expr/Cargo.toml | 4 +
native/spark-expr/benches/make_decimal.rs | 164 +++++++++++++++
.../src/math_funcs/internal/make_decimal.rs | 219 ++++++++++++++++++++-
4 files changed, 503 insertions(+), 65 deletions(-)
diff --git a/native/core/src/execution/columnar_to_row.rs
b/native/core/src/execution/columnar_to_row.rs
index 14e115cba0..a1321a9def 100644
--- a/native/core/src/execution/columnar_to_row.rs
+++ b/native/core/src/execution/columnar_to_row.rs
@@ -37,10 +37,11 @@
use crate::errors::{CometError, CometResult};
use arrow::array::types::{
- ArrowDictionaryKeyType, Int16Type, Int32Type, Int64Type, Int8Type,
UInt16Type, UInt32Type,
- UInt64Type, UInt8Type,
+ ArrowDictionaryKeyType, Decimal128Type, Int16Type, Int32Type, Int64Type,
Int8Type, UInt16Type,
+ UInt32Type, UInt64Type, UInt8Type,
};
use arrow::array::*;
+use arrow::compute::kernels::arity::unary;
use arrow::compute::{cast_with_options, CastOptions};
use arrow::datatypes::{ArrowNativeType, DataType, TimeUnit};
use std::sync::Arc;
@@ -1055,14 +1056,10 @@ impl ColumnarToRowContext {
// Parquet stores small-precision decimals as Int32 for
efficiency, and the
// reader may surface them as the physical Int32 type. The
value is already
// scaled (e.g., -1 means -0.01 for scale 2). Reinterpret (not
cast) to
- // Decimal128 preserving the value.
- let int_array =
array.as_any().downcast_ref::<Int32Array>().ok_or_else(|| {
- CometError::Internal("Failed to downcast to
Int32Array".to_string())
- })?;
- let decimal_array: Decimal128Array = int_array
- .iter()
- .map(|v| v.map(|x| x as i128))
- .collect::<Decimal128Array>()
+ // Decimal128 preserving the value. `arity::unary` widens the
value and reuses
+ // the input null buffer zero-copy (an Arrow cast would
rescale the value).
+ let int_array = array.as_primitive::<Int32Type>();
+ let decimal_array = unary::<_, _, Decimal128Type>(int_array,
|x| x as i128)
.with_precision_and_scale(*precision, *scale)
.map_err(|e| {
CometError::Internal(format!("Invalid decimal
precision/scale: {}", e))
@@ -1071,13 +1068,8 @@ impl ColumnarToRowContext {
}
(DataType::Int64, DataType::Decimal128(precision, scale)) => {
// Same as Int32 but for medium-precision decimals stored as
Int64.
- let int_array =
array.as_any().downcast_ref::<Int64Array>().ok_or_else(|| {
- CometError::Internal("Failed to downcast to
Int64Array".to_string())
- })?;
- let decimal_array: Decimal128Array = int_array
- .iter()
- .map(|v| v.map(|x| x as i128))
- .collect::<Decimal128Array>()
+ let int_array = array.as_primitive::<Int64Type>();
+ let decimal_array = unary::<_, _, Decimal128Type>(int_array,
|x| x as i128)
.with_precision_and_scale(*precision, *scale)
.map_err(|e| {
CometError::Internal(format!("Invalid decimal
precision/scale: {}", e))
@@ -2580,14 +2572,10 @@ mod tests {
#[test]
fn test_convert_int32_to_decimal128() {
- // Test that Int32 arrays are correctly cast to Decimal128 when schema
expects Decimal128.
- // This can happen when the parquet reader surfaces small-precision
decimals as Int32.
-
- // Create an Int32 array representing decimals: [-1, -2, -3] which at
scale 2 means
- // [-0.01, -0.02, -0.03]
- let int_array: ArrayRef = Arc::new(Int32Array::from(vec![-1i32, -2,
-3]));
+ // Int32 → Decimal128 reinterpret. Values [-1, null, -3] at scale 2
mean
+ // [-0.01, null, -0.03]. The null covers the arity::unary null-buffer
path.
+ let int_array: ArrayRef = Arc::new(Int32Array::from(vec![Some(-1i32),
None, Some(-3)]));
- // Schema expects Decimal128(5, 2)
let schema = vec![DataType::Decimal128(5, 2)];
let mut ctx = ColumnarToRowContext::new(schema, 100);
@@ -2598,32 +2586,74 @@ mod tests {
assert_eq!(offsets.len(), 3);
assert_eq!(lengths.len(), 3);
- // Verify the decimal values are correct after casting
- // Fixed-width decimal is stored directly in the 8-byte field slot
+ // Row layout: [8-byte null bitset][8-byte field slot]. Column 0's
null bit
+ // is bit 0 of the bitset.
unsafe {
- for (i, expected) in [-1i64, -2, -3].iter().enumerate() {
- let row =
- std::slice::from_raw_parts(ptr.add(offsets[i] as usize),
lengths[i] as usize);
- // Field value starts at offset 8 (after null bitset)
- let value = i64::from_le_bytes(row[8..16].try_into().unwrap());
- assert_eq!(
- value, *expected,
- "Row {} should have value {}, got {}",
- i, expected, value
- );
- }
+ let row0 =
+ std::slice::from_raw_parts(ptr.add(offsets[0] as usize),
lengths[0] as usize);
+ let null_bitset_0 =
i64::from_le_bytes(row0[0..8].try_into().unwrap());
+ assert_eq!(null_bitset_0 & 1, 0, "row 0 should not be null");
+ assert_eq!(i64::from_le_bytes(row0[8..16].try_into().unwrap()),
-1);
+
+ let row1 =
+ std::slice::from_raw_parts(ptr.add(offsets[1] as usize),
lengths[1] as usize);
+ let null_bitset_1 =
i64::from_le_bytes(row1[0..8].try_into().unwrap());
+ assert_eq!(null_bitset_1 & 1, 1, "row 1 should be null");
+
+ let row2 =
+ std::slice::from_raw_parts(ptr.add(offsets[2] as usize),
lengths[2] as usize);
+ let null_bitset_2 =
i64::from_le_bytes(row2[0..8].try_into().unwrap());
+ assert_eq!(null_bitset_2 & 1, 0, "row 2 should not be null");
+ assert_eq!(i64::from_le_bytes(row2[8..16].try_into().unwrap()),
-3);
}
}
#[test]
- fn test_convert_int64_to_decimal128() {
- // Test that Int64 arrays are correctly cast to Decimal128 when schema
expects Decimal128.
- // This can happen when the parquet reader surfaces medium-precision
decimals as Int64.
+ fn test_convert_int32_to_decimal128_sliced() {
+ // Sliced input must reinterpret correctly. Arrays imported over FFI
often
+ // carry a non-zero offset; both the value widening and the null buffer
+ // traversal must honor it.
+ let full = Int32Array::from(vec![
+ Some(-99i32), // discarded prefix
+ Some(-98),
+ Some(-1), // start of sliced portion
+ None,
+ Some(-3),
+ Some(-97), // discarded suffix
+ ]);
+ let sliced: ArrayRef = Arc::new(full.slice(2, 3));
+
+ let schema = vec![DataType::Decimal128(5, 2)];
+ let mut ctx = ColumnarToRowContext::new(schema, 100);
+ let arrays = vec![sliced];
+ let (ptr, offsets, lengths) = ctx.convert(&arrays, 3).unwrap();
+
+ assert!(!ptr.is_null());
+ assert_eq!(offsets.len(), 3);
- // Create an Int64 array representing decimals
- let int_array: ArrayRef = Arc::new(Int64Array::from(vec![-100i64,
-200, -300]));
+ unsafe {
+ let row0 =
+ std::slice::from_raw_parts(ptr.add(offsets[0] as usize),
lengths[0] as usize);
+ assert_eq!(i64::from_le_bytes(row0[0..8].try_into().unwrap()) & 1,
0);
+ assert_eq!(i64::from_le_bytes(row0[8..16].try_into().unwrap()),
-1);
+
+ let row1 =
+ std::slice::from_raw_parts(ptr.add(offsets[1] as usize),
lengths[1] as usize);
+ assert_eq!(i64::from_le_bytes(row1[0..8].try_into().unwrap()) & 1,
1);
+
+ let row2 =
+ std::slice::from_raw_parts(ptr.add(offsets[2] as usize),
lengths[2] as usize);
+ assert_eq!(i64::from_le_bytes(row2[0..8].try_into().unwrap()) & 1,
0);
+ assert_eq!(i64::from_le_bytes(row2[8..16].try_into().unwrap()),
-3);
+ }
+ }
+
+ #[test]
+ fn test_convert_int64_to_decimal128() {
+ // Int64 → Decimal128 reinterpret. Values [-100, null, -300] with a
null in
+ // the middle to exercise the arity::unary null-buffer path.
+ let int_array: ArrayRef =
Arc::new(Int64Array::from(vec![Some(-100i64), None, Some(-300)]));
- // Schema expects Decimal128(10, 2)
let schema = vec![DataType::Decimal128(10, 2)];
let mut ctx = ColumnarToRowContext::new(schema, 100);
@@ -2634,19 +2664,60 @@ mod tests {
assert_eq!(offsets.len(), 3);
assert_eq!(lengths.len(), 3);
- // Verify the decimal values are correct after casting
unsafe {
- for (i, expected) in [-100i64, -200, -300].iter().enumerate() {
- let row =
- std::slice::from_raw_parts(ptr.add(offsets[i] as usize),
lengths[i] as usize);
- // Field value starts at offset 8 (after null bitset)
- let value = i64::from_le_bytes(row[8..16].try_into().unwrap());
- assert_eq!(
- value, *expected,
- "Row {} should have value {}, got {}",
- i, expected, value
- );
- }
+ let row0 =
+ std::slice::from_raw_parts(ptr.add(offsets[0] as usize),
lengths[0] as usize);
+ let null_bitset_0 =
i64::from_le_bytes(row0[0..8].try_into().unwrap());
+ assert_eq!(null_bitset_0 & 1, 0, "row 0 should not be null");
+ assert_eq!(i64::from_le_bytes(row0[8..16].try_into().unwrap()),
-100);
+
+ let row1 =
+ std::slice::from_raw_parts(ptr.add(offsets[1] as usize),
lengths[1] as usize);
+ let null_bitset_1 =
i64::from_le_bytes(row1[0..8].try_into().unwrap());
+ assert_eq!(null_bitset_1 & 1, 1, "row 1 should be null");
+
+ let row2 =
+ std::slice::from_raw_parts(ptr.add(offsets[2] as usize),
lengths[2] as usize);
+ let null_bitset_2 =
i64::from_le_bytes(row2[0..8].try_into().unwrap());
+ assert_eq!(null_bitset_2 & 1, 0, "row 2 should not be null");
+ assert_eq!(i64::from_le_bytes(row2[8..16].try_into().unwrap()),
-300);
+ }
+ }
+
+ #[test]
+ fn test_convert_int64_to_decimal128_sliced() {
+ let full = Int64Array::from(vec![
+ Some(-9999i64),
+ Some(-9998),
+ Some(-100),
+ None,
+ Some(-300),
+ Some(-9997),
+ ]);
+ let sliced: ArrayRef = Arc::new(full.slice(2, 3));
+
+ let schema = vec![DataType::Decimal128(10, 2)];
+ let mut ctx = ColumnarToRowContext::new(schema, 100);
+ let arrays = vec![sliced];
+ let (ptr, offsets, lengths) = ctx.convert(&arrays, 3).unwrap();
+
+ assert!(!ptr.is_null());
+ assert_eq!(offsets.len(), 3);
+
+ unsafe {
+ let row0 =
+ std::slice::from_raw_parts(ptr.add(offsets[0] as usize),
lengths[0] as usize);
+ assert_eq!(i64::from_le_bytes(row0[0..8].try_into().unwrap()) & 1,
0);
+ assert_eq!(i64::from_le_bytes(row0[8..16].try_into().unwrap()),
-100);
+
+ let row1 =
+ std::slice::from_raw_parts(ptr.add(offsets[1] as usize),
lengths[1] as usize);
+ assert_eq!(i64::from_le_bytes(row1[0..8].try_into().unwrap()) & 1,
1);
+
+ let row2 =
+ std::slice::from_raw_parts(ptr.add(offsets[2] as usize),
lengths[2] as usize);
+ assert_eq!(i64::from_le_bytes(row2[0..8].try_into().unwrap()) & 1,
0);
+ assert_eq!(i64::from_le_bytes(row2[8..16].try_into().unwrap()),
-300);
}
}
}
diff --git a/native/spark-expr/Cargo.toml b/native/spark-expr/Cargo.toml
index cd7cd89849..2333d2f0e6 100644
--- a/native/spark-expr/Cargo.toml
+++ b/native/spark-expr/Cargo.toml
@@ -215,6 +215,10 @@ harness = false
name = "unscaled_value"
harness = false
+[[bench]]
+name = "make_decimal"
+harness = false
+
[[bench]]
name = "cast_int_to_decimal"
harness = false
\ No newline at end of file
diff --git a/native/spark-expr/benches/make_decimal.rs
b/native/spark-expr/benches/make_decimal.rs
new file mode 100644
index 0000000000..c1cc7fef1d
--- /dev/null
+++ b/native/spark-expr/benches/make_decimal.rs
@@ -0,0 +1,164 @@
+// 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.
+
+use arrow::array::{ArrayRef, Int64Array};
+use arrow::datatypes::DataType;
+use criterion::{criterion_group, criterion_main, Criterion};
+use datafusion::physical_plan::ColumnarValue;
+use datafusion_comet_spark_expr::spark_make_decimal;
+use std::hint::black_box;
+use std::sync::Arc;
+
+// Target Decimal128(18, 2): the widest precision DecimalAggregates produces
for MakeDecimal.
+// At precision 18 every i64 fits, so every value in these benches is in the
no-overflow
+// common path.
+const NO_OVERFLOW_PRECISION: u8 = 18;
+const NO_OVERFLOW_SCALE: i8 = 2;
+
+// Narrow target used only for the overflow shapes. Values with |v| >= 1000 do
not fit.
+const OVERFLOW_PRECISION: u8 = 3;
+const OVERFLOW_SCALE: i8 = 0;
+const OVERFLOW_VALUE: i64 = 1000;
+
+/// Build an Int64 column of `rows` rows, with every `null_every`-th row null
+/// (`null_every == 0` means no nulls). Values always fit Decimal128(18, 2).
+fn create_int64_array(rows: usize, null_every: usize) -> ArrayRef {
+ let arr: Int64Array = (0..rows)
+ .map(|i| {
+ if null_every != 0 && i % null_every == 0 {
+ None
+ } else {
+ Some((i as i64 % 100_000) * 100)
+ }
+ })
+ .collect();
+ Arc::new(arr)
+}
+
+/// Build an Int64 column where every row overflows Decimal128(3, 0).
+fn create_all_overflow_array(rows: usize) -> ArrayRef {
+ let arr: Int64Array = (0..rows).map(|_| Some(OVERFLOW_VALUE)).collect();
+ Arc::new(arr)
+}
+
+/// Build an Int64 column where only the last row overflows Decimal128(3, 0).
+fn create_last_row_overflow_array(rows: usize) -> ArrayRef {
+ let arr: Int64Array = (0..rows)
+ .map(|i| {
+ if i + 1 == rows {
+ Some(OVERFLOW_VALUE)
+ } else {
+ Some((i as i64 % 999) + 1)
+ }
+ })
+ .collect();
+ Arc::new(arr)
+}
+
+fn criterion_benchmark(c: &mut Criterion) {
+ let rows = 8192;
+ let no_overflow_target = DataType::Decimal128(NO_OVERFLOW_PRECISION,
NO_OVERFLOW_SCALE);
+ let overflow_target = DataType::Decimal128(OVERFLOW_PRECISION,
OVERFLOW_SCALE);
+
+ let no_nulls = create_int64_array(rows, 0);
+ let sparse_nulls = create_int64_array(rows, 10);
+ let dense_nulls = create_int64_array(rows, 2);
+ let all_overflow = create_all_overflow_array(rows);
+ let last_overflow = create_last_row_overflow_array(rows);
+
+ // Success / non-ANSI paths: unwrap like the neighbouring expression
benches.
+ // Two-arg helpers match `unscaled_value.rs` (name + array;
target/`fail_on_error` captured).
+ {
+ let target = no_overflow_target.clone();
+ let mut bench = |name: &str, arr: &ArrayRef| {
+ let args = vec![ColumnarValue::Array(Arc::clone(arr))];
+ let target = target.clone();
+ c.bench_function(name, move |b| {
+ b.iter(|| {
+ black_box(
+ spark_make_decimal(black_box(&args),
black_box(&target), false).unwrap(),
+ )
+ })
+ });
+ };
+ bench("spark_make_decimal: no nulls", &no_nulls);
+ bench("spark_make_decimal: sparse nulls", &sparse_nulls);
+ bench("spark_make_decimal: dense nulls", &dense_nulls);
+ }
+ {
+ let target = no_overflow_target.clone();
+ let mut bench = |name: &str, arr: &ArrayRef| {
+ let args = vec![ColumnarValue::Array(Arc::clone(arr))];
+ let target = target.clone();
+ c.bench_function(name, move |b| {
+ b.iter(|| {
+ black_box(
+ spark_make_decimal(black_box(&args),
black_box(&target), true).unwrap(),
+ )
+ })
+ });
+ };
+ bench("spark_make_decimal: ansi no nulls", &no_nulls);
+ bench("spark_make_decimal: ansi sparse nulls", &sparse_nulls);
+ bench("spark_make_decimal: ansi dense nulls", &dense_nulls);
+ }
+ {
+ // Non-ANSI overflow: short-circuiting scan then
`null_if_overflow_precision`.
+ let target = overflow_target.clone();
+ let mut bench = |name: &str, arr: &ArrayRef| {
+ let args = vec![ColumnarValue::Array(Arc::clone(arr))];
+ let target = target.clone();
+ c.bench_function(name, move |b| {
+ b.iter(|| {
+ black_box(
+ spark_make_decimal(black_box(&args),
black_box(&target), false).unwrap(),
+ )
+ })
+ });
+ };
+ bench("spark_make_decimal: overflow all rows", &all_overflow);
+ bench("spark_make_decimal: overflow last row", &last_overflow);
+ }
+
+ // ANSI overflow raises; black-box the Result (same idea as check_overflow
only
+ // unwrapping non-error paths, but here the error path itself is the shape
under test).
+ let ansi_last_args =
vec![ColumnarValue::Array(Arc::clone(&last_overflow))];
+ let ansi_last_target = overflow_target.clone();
+ c.bench_function("spark_make_decimal: ansi overflow last row", |b| {
+ b.iter(|| {
+ black_box(spark_make_decimal(
+ black_box(&ansi_last_args),
+ black_box(&ansi_last_target),
+ true,
+ ))
+ })
+ });
+
+ let ansi_all_args = vec![ColumnarValue::Array(Arc::clone(&all_overflow))];
+ c.bench_function("spark_make_decimal: ansi overflow all rows", |b| {
+ b.iter(|| {
+ black_box(spark_make_decimal(
+ black_box(&ansi_all_args),
+ black_box(&overflow_target),
+ true,
+ ))
+ })
+ });
+}
+
+criterion_group!(benches, criterion_benchmark);
+criterion_main!(benches);
diff --git a/native/spark-expr/src/math_funcs/internal/make_decimal.rs
b/native/spark-expr/src/math_funcs/internal/make_decimal.rs
index 1961ffd66b..13d677999a 100644
--- a/native/spark-expr/src/math_funcs/internal/make_decimal.rs
+++ b/native/spark-expr/src/math_funcs/internal/make_decimal.rs
@@ -17,10 +17,11 @@
use crate::error::decimal_overflow_error;
use crate::math_funcs::utils::get_precision_scale;
+use arrow::compute::kernels::arity::unary;
use arrow::datatypes::DataType;
use arrow::{
- array::{AsArray, Decimal128Builder},
- datatypes::{validate_decimal_precision, Int64Type},
+ array::{AsArray, Decimal128Array},
+ datatypes::{validate_decimal_precision, Decimal128Type, DecimalType,
Int64Type},
};
use datafusion::common::{internal_err, DataFusionError, Result as
DataFusionResult, ScalarValue};
use datafusion::physical_plan::ColumnarValue;
@@ -45,15 +46,34 @@ pub fn spark_make_decimal(
ColumnarValue::Array(a) => match a.data_type() {
DataType::Int64 => {
let arr = a.as_primitive::<Int64Type>();
- let mut result = Decimal128Builder::new();
- for v in arr.into_iter() {
- result.append_option(long_to_decimal(v, precision, scale,
fail_on_error)?)
- }
let result_type = DataType::Decimal128(precision, scale);
- Ok(ColumnarValue::Array(Arc::new(
- result.finish().with_data_type(result_type),
- )))
+ // The Int64 is already the unscaled Decimal128 value; this
widens the bits
+ // (an Arrow Int64->Decimal cast would rescale). Infallible so
it vectorizes.
+ let widened: Decimal128Array = unary::<_, _,
Decimal128Type>(arr, |v| v as i128);
+
+ // `.iter().flatten()` skips null slots so garbage under a
null cannot
+ // trigger a false overflow. `find` short-circuits like
`.all(is_valid)`
+ // while also handing back the value needed for the ANSI error
message.
+ let first_offender = widened
+ .iter()
+ .flatten()
+ .find(|v| !Decimal128Type::is_valid_decimal_precision(*v,
precision));
+
+ let result = match (first_offender, fail_on_error) {
+ // No overflow: attach metadata.
`with_precision_and_scale` would rescan.
+ (None, _) => widened.with_data_type(result_type),
+ (Some(v), true) => {
+ return
Err(DataFusionError::External(Box::new(decimal_overflow_error(
+ v, precision, scale,
+ ))));
+ }
+ (Some(_), false) => widened
+ .null_if_overflow_precision(precision)
+ .with_data_type(result_type),
+ };
+
+ Ok(ColumnarValue::Array(Arc::new(result)))
}
av => internal_err!("Expected Int64 but found {av:?}"),
},
@@ -85,7 +105,7 @@ fn long_to_decimal(
#[cfg(test)]
mod tests {
use super::*;
- use arrow::array::{Array, Int64Array};
+ use arrow::array::{Array, Decimal128Array, Int64Array};
fn overflow_args() -> [ColumnarValue; 1] {
// 123456 does not fit Decimal128(3, 0)
@@ -106,6 +126,30 @@ mod tests {
);
}
+ #[test]
+ fn test_array_overflow_reports_first_offending_value() {
+ // Two distinct overflowing values with a valid value in front. The
+ // `.iter().flatten().find(...)` scan walks rows in index order, so
the reported
+ // error must reference the FIRST overflow (111111), not the later one
(222222).
+ // This locks the ordering semantics rather than merely "some error
occurred".
+ let args = [ColumnarValue::Array(Arc::new(Int64Array::from(vec![
+ Some(99),
+ Some(111111),
+ Some(222222),
+ ])))];
+ let err = spark_make_decimal(&args, &DataType::Decimal128(3, 0), true)
+ .expect_err("overflow should error when fail_on_error is set");
+ let msg = err.to_string();
+ assert!(
+ msg.contains("111111"),
+ "should report first overflow: {msg}"
+ );
+ assert!(
+ !msg.contains("222222"),
+ "should not report later overflow: {msg}"
+ );
+ }
+
#[test]
fn test_array_overflow_nulls_when_not_fail_on_error() {
let result = spark_make_decimal(&overflow_args(),
&DataType::Decimal128(3, 0), false)
@@ -139,4 +183,159 @@ mod tests {
};
assert!(v.is_none());
}
+
+ #[test]
+ fn test_scalar_valid_value() {
+ // Locks the happy path for the scalar branch: an in-range value
round-trips
+ // to the correct Decimal128 with matching precision/scale. Existing
scalar
+ // tests only cover overflow and null input.
+ let args = [ColumnarValue::Scalar(ScalarValue::Int64(Some(999)))];
+ let result = spark_make_decimal(&args, &DataType::Decimal128(3, 0),
true)
+ .expect("in-range scalar should succeed");
+ let ColumnarValue::Scalar(ScalarValue::Decimal128(v, 3, 0)) = result
else {
+ panic!("expected decimal scalar result")
+ };
+ assert_eq!(v, Some(999));
+ }
+
+ #[test]
+ fn test_scalar_overflow_nulls_when_not_fail_on_error() {
+ // Covers `long_to_decimal`'s `Err(_) => Ok(None)` branch through the
scalar
+ // path. The array path exercises this branch indirectly via
`null_if_overflow_precision`;
+ // the scalar branch has its own code and was previously untested.
+ let args = [ColumnarValue::Scalar(ScalarValue::Int64(Some(123456)))];
+ let result = spark_make_decimal(&args, &DataType::Decimal128(3, 0),
false)
+ .expect("scalar overflow without fail_on_error should return
null");
+ let ColumnarValue::Scalar(ScalarValue::Decimal128(v, 3, 0)) = result
else {
+ panic!("expected decimal scalar result")
+ };
+ assert_eq!(v, None);
+ }
+
+ #[test]
+ fn test_array_no_overflow_fast_path() {
+ // Common path: no overflow, no nulls. Values pass through unchanged
and the
+ // output carries the target Decimal128 type. Locks the (None, _) arm
that
+ // existing overflow tests never touch.
+ let args = [ColumnarValue::Array(Arc::new(Int64Array::from(vec![
+ Some(1i64),
+ Some(50),
+ Some(999),
+ ])))];
+ let result = spark_make_decimal(&args, &DataType::Decimal128(3, 0),
false)
+ .expect("no overflow should succeed");
+ let ColumnarValue::Array(array) = result else {
+ panic!("expected array result")
+ };
+ assert_eq!(array.data_type(), &DataType::Decimal128(3, 0));
+ assert_eq!(array.null_count(), 0);
+ let decimals =
array.as_any().downcast_ref::<Decimal128Array>().unwrap();
+ assert_eq!(decimals.value(0), 1);
+ assert_eq!(decimals.value(1), 50);
+ assert_eq!(decimals.value(2), 999);
+ }
+
+ #[test]
+ fn test_array_negative_overflow_nulls() {
+ // `is_valid_decimal_precision` checks both bounds; -1000 is below the
+ // Decimal128(3, 0) lower bound (-999). Guards the negative branch that
+ // existing overflow tests (all positive) do not cover.
+ let args = [ColumnarValue::Array(Arc::new(Int64Array::from(vec![
+ Some(-1000i64),
+ Some(5),
+ ])))];
+ let result = spark_make_decimal(&args, &DataType::Decimal128(3, 0),
false)
+ .expect("negative overflow should become null");
+ let ColumnarValue::Array(array) = result else {
+ panic!("expected array result")
+ };
+ assert!(array.is_null(0), "negative overflow should be nulled");
+ assert!(array.is_valid(1));
+ }
+
+ #[test]
+ fn test_array_null_slot_garbage_not_scanned() {
+ // Regression guard for the `.iter().flatten()` decision. `unary`
widens
+ // every slot including nulls, so the null slot's underlying storage
can
+ // hold arbitrary bits. If the scan ever switched to
`.values().iter()`,
+ // i64::MAX under the null would falsely trip Decimal128(3, 0)
overflow.
+ use arrow::buffer::{NullBuffer, ScalarBuffer};
+
+ let values = ScalarBuffer::from(vec![10i64, i64::MAX, 20i64]);
+ let nulls = NullBuffer::from(vec![true, false, true]);
+ let arr = Int64Array::new(values, Some(nulls));
+ let args = [ColumnarValue::Array(Arc::new(arr))];
+
+ // Non-ANSI: no overflow detected, null slot stays null (not
double-nullified).
+ let result = spark_make_decimal(&args, &DataType::Decimal128(3, 0),
false)
+ .expect("null slot garbage must not trigger overflow");
+ let ColumnarValue::Array(array) = result else {
+ panic!("expected array result")
+ };
+ assert!(array.is_valid(0));
+ assert!(array.is_null(1));
+ assert!(array.is_valid(2));
+
+ // ANSI: also must not raise on garbage hidden behind a null.
+ spark_make_decimal(&args, &DataType::Decimal128(3, 0), true)
+ .expect("ANSI mode must not error on null slot garbage");
+ }
+
+ #[test]
+ fn test_array_boundary_precision() {
+ // 999 is exactly the max for Decimal128(3, 0); 1000 is over by one.
+ // Pins the off-by-one on `is_valid_decimal_precision`.
+ let args = [ColumnarValue::Array(Arc::new(Int64Array::from(vec![
+ Some(999i64),
+ Some(1000),
+ ])))];
+ let result = spark_make_decimal(&args, &DataType::Decimal128(3, 0),
false)
+ .expect("boundary case should not error in non-ANSI");
+ let ColumnarValue::Array(array) = result else {
+ panic!("expected array result")
+ };
+ assert!(array.is_valid(0));
+ assert!(array.is_null(1));
+ }
+
+ #[test]
+ fn test_scalar_boundary_precision() {
+ // Same off-by-one as `test_array_boundary_precision`, but through the
scalar path
+ // (`validate_decimal_precision`). Locks the two Arrow entry points
together.
+ let ok = [ColumnarValue::Scalar(ScalarValue::Int64(Some(999)))];
+ let result = spark_make_decimal(&ok, &DataType::Decimal128(3, 0),
false)
+ .expect("999 should fit Decimal128(3, 0)");
+ let ColumnarValue::Scalar(ScalarValue::Decimal128(v, 3, 0)) = result
else {
+ panic!("expected decimal scalar result")
+ };
+ assert_eq!(v, Some(999));
+
+ let over = [ColumnarValue::Scalar(ScalarValue::Int64(Some(1000)))];
+ let result = spark_make_decimal(&over, &DataType::Decimal128(3, 0),
false)
+ .expect("1000 should null without fail_on_error");
+ let ColumnarValue::Scalar(ScalarValue::Decimal128(v, 3, 0)) = result
else {
+ panic!("expected decimal scalar result")
+ };
+ assert_eq!(v, None);
+ }
+
+ #[test]
+ fn test_array_all_null() {
+ // .iter().flatten() on an all-null array yields nothing, so `find`
returns
+ // None and the fast path is taken. Locks that the all-null mask and
target
+ // type are preserved.
+ let args = [ColumnarValue::Array(Arc::new(Int64Array::from(vec![
+ None::<i64>,
+ None,
+ None,
+ ])))];
+ let result = spark_make_decimal(&args, &DataType::Decimal128(3, 0),
false)
+ .expect("all-null should succeed");
+ let ColumnarValue::Array(array) = result else {
+ panic!("expected array result")
+ };
+ assert_eq!(array.data_type(), &DataType::Decimal128(3, 0));
+ assert_eq!(array.len(), 3);
+ assert_eq!(array.null_count(), 3);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
