From ec65ecbfa92b0e086ccb51ecf9118efc784c8570 Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Thu, 16 Jul 2026 18:03:30 +0900 Subject: [PATCH 1/7] Use arrow LUTs for min/max of scalarvals decimals, add dec32+dec64 too Since we already have access to these constant lookup tables, its more efficient to index into them rather than calculating via `checked_pow()` at runtime. --- datafusion/common/src/scalar/mod.rs | 73 +++++++++++++---------------- 1 file changed, 32 insertions(+), 41 deletions(-) diff --git a/datafusion/common/src/scalar/mod.rs b/datafusion/common/src/scalar/mod.rs index ddfe32edd41cc..cc282f2fdafcc 100644 --- a/datafusion/common/src/scalar/mod.rs +++ b/datafusion/common/src/scalar/mod.rs @@ -83,10 +83,14 @@ use arrow::datatypes::{ Decimal32Type, Decimal64Type, Decimal128Type, Decimal256Type, DecimalType, Field, FieldRef, Float32Type, Int8Type, Int16Type, Int32Type, Int64Type, IntervalDayTime, IntervalDayTimeType, IntervalMonthDayNano, IntervalMonthDayNanoType, IntervalUnit, - IntervalYearMonthType, RunEndIndexType, TimeUnit, TimestampMicrosecondType, - TimestampMillisecondType, TimestampNanosecondType, TimestampSecondType, UInt8Type, - UInt16Type, UInt32Type, UInt64Type, UnionFields, UnionMode, i256, - validate_decimal_precision_and_scale, + IntervalYearMonthType, MAX_DECIMAL32_FOR_EACH_PRECISION, + MAX_DECIMAL64_FOR_EACH_PRECISION, MAX_DECIMAL128_FOR_EACH_PRECISION, + MAX_DECIMAL256_FOR_EACH_PRECISION, MIN_DECIMAL32_FOR_EACH_PRECISION, + MIN_DECIMAL64_FOR_EACH_PRECISION, MIN_DECIMAL128_FOR_EACH_PRECISION, + MIN_DECIMAL256_FOR_EACH_PRECISION, RunEndIndexType, TimeUnit, + TimestampMicrosecondType, TimestampMillisecondType, TimestampNanosecondType, + TimestampSecondType, UInt8Type, UInt16Type, UInt32Type, UInt64Type, UnionFields, + UnionMode, i256, validate_decimal_precision_and_scale, }; use arrow::util::display::{ArrayFormatter, FormatOptions, array_value_to_string}; use cache::{get_or_create_cached_key_array, get_or_create_cached_null_array}; @@ -5053,28 +5057,21 @@ impl ScalarValue { DataType::Float16 => Some(ScalarValue::Float16(Some(f16::NEG_INFINITY))), DataType::Float32 => Some(ScalarValue::Float32(Some(f32::NEG_INFINITY))), DataType::Float64 => Some(ScalarValue::Float64(Some(f64::NEG_INFINITY))), + DataType::Decimal32(precision, scale) => { + let min = MIN_DECIMAL32_FOR_EACH_PRECISION[*precision as usize]; + Some(ScalarValue::Decimal32(Some(min), *precision, *scale)) + } + DataType::Decimal64(precision, scale) => { + let min = MIN_DECIMAL64_FOR_EACH_PRECISION[*precision as usize]; + Some(ScalarValue::Decimal64(Some(min), *precision, *scale)) + } DataType::Decimal128(precision, scale) => { - // For decimal, min is -10^(precision-scale) + 10^(-scale) - // But for simplicity, we use the minimum i128 value that fits the precision - let max_digits = 10_i128.pow(*precision as u32) - 1; - Some(ScalarValue::Decimal128( - Some(-max_digits), - *precision, - *scale, - )) + let min = MIN_DECIMAL128_FOR_EACH_PRECISION[*precision as usize]; + Some(ScalarValue::Decimal128(Some(min), *precision, *scale)) } DataType::Decimal256(precision, scale) => { - // Similar to Decimal128 but with i256 - // For now, use a large negative value - let max_digits = i256::from_i128(10_i128) - .checked_pow(*precision as u32) - .and_then(|v| v.checked_sub(i256::from_i128(1))) - .unwrap_or(i256::MAX); - Some(ScalarValue::Decimal256( - Some(max_digits.neg_wrapping()), - *precision, - *scale, - )) + let min = MIN_DECIMAL256_FOR_EACH_PRECISION[*precision as usize]; + Some(ScalarValue::Decimal256(Some(min), *precision, *scale)) } DataType::Date32 => Some(ScalarValue::Date32(Some(i32::MIN))), DataType::Date64 => Some(ScalarValue::Date64(Some(i64::MIN))), @@ -5149,27 +5146,21 @@ impl ScalarValue { DataType::Float16 => Some(ScalarValue::Float16(Some(f16::INFINITY))), DataType::Float32 => Some(ScalarValue::Float32(Some(f32::INFINITY))), DataType::Float64 => Some(ScalarValue::Float64(Some(f64::INFINITY))), + DataType::Decimal32(precision, scale) => { + let max = MAX_DECIMAL32_FOR_EACH_PRECISION[*precision as usize]; + Some(ScalarValue::Decimal32(Some(max), *precision, *scale)) + } + DataType::Decimal64(precision, scale) => { + let max = MAX_DECIMAL64_FOR_EACH_PRECISION[*precision as usize]; + Some(ScalarValue::Decimal64(Some(max), *precision, *scale)) + } DataType::Decimal128(precision, scale) => { - // For decimal, max is 10^(precision-scale) - 10^(-scale) - // But for simplicity, we use the maximum i128 value that fits the precision - let max_digits = 10_i128.pow(*precision as u32) - 1; - Some(ScalarValue::Decimal128( - Some(max_digits), - *precision, - *scale, - )) + let max = MAX_DECIMAL128_FOR_EACH_PRECISION[*precision as usize]; + Some(ScalarValue::Decimal128(Some(max), *precision, *scale)) } DataType::Decimal256(precision, scale) => { - // Similar to Decimal128 but with i256 - let max_digits = i256::from_i128(10_i128) - .checked_pow(*precision as u32) - .and_then(|v| v.checked_sub(i256::from_i128(1))) - .unwrap_or(i256::MAX); - Some(ScalarValue::Decimal256( - Some(max_digits), - *precision, - *scale, - )) + let max = MAX_DECIMAL256_FOR_EACH_PRECISION[*precision as usize]; + Some(ScalarValue::Decimal256(Some(max), *precision, *scale)) } DataType::Date32 => Some(ScalarValue::Date32(Some(i32::MAX))), DataType::Date64 => Some(ScalarValue::Date64(Some(i64::MAX))), From 658980d6674c2c0c8bed27c0dca03db97acb5425 Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Thu, 16 Jul 2026 18:04:15 +0900 Subject: [PATCH 2/7] Remove support for `-1` from unsigned scalar ints There's a side-effect of some tests failing so I removed them. I think this is fine since it doesn't make sense to have a `ScalarValue` method where we request a value of a certain type but can get a different value type out. --- datafusion/common/src/scalar/mod.rs | 8 ++--- .../simplify_expressions/expr_simplifier.rs | 35 ------------------- 2 files changed, 4 insertions(+), 39 deletions(-) diff --git a/datafusion/common/src/scalar/mod.rs b/datafusion/common/src/scalar/mod.rs index cc282f2fdafcc..4f8a3c24eb4a5 100644 --- a/datafusion/common/src/scalar/mod.rs +++ b/datafusion/common/src/scalar/mod.rs @@ -1862,10 +1862,10 @@ impl ScalarValue { /// Create a negative one value in the given type. pub fn new_negative_one(datatype: &DataType) -> Result { Ok(match datatype { - DataType::Int8 | DataType::UInt8 => ScalarValue::Int8(Some(-1)), - DataType::Int16 | DataType::UInt16 => ScalarValue::Int16(Some(-1)), - DataType::Int32 | DataType::UInt32 => ScalarValue::Int32(Some(-1)), - DataType::Int64 | DataType::UInt64 => ScalarValue::Int64(Some(-1)), + DataType::Int8 => ScalarValue::Int8(Some(-1)), + DataType::Int16 => ScalarValue::Int16(Some(-1)), + DataType::Int32 => ScalarValue::Int32(Some(-1)), + DataType::Int64 => ScalarValue::Int64(Some(-1)), DataType::Float16 => ScalarValue::Float16(Some(f16::NEG_ONE)), DataType::Float32 => ScalarValue::Float32(Some(-1.0)), DataType::Float64 => ScalarValue::Float64(Some(-1.0)), diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs index 39c8541b51b2f..9abc3faa2b32b 100644 --- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs @@ -3062,17 +3062,6 @@ mod tests { #[test] fn test_simplify_negated_bitwise_and() { - // !c4 & c4 --> 0 - let expr = (-col("c4_non_null")) & col("c4_non_null"); - let expected = lit(0u32); - - assert_eq!(simplify(expr), expected); - // c4 & !c4 --> 0 - let expr = col("c4_non_null") & (-col("c4_non_null")); - let expected = lit(0u32); - - assert_eq!(simplify(expr), expected); - // !c3 & c3 --> 0 let expr = (-col("c3_non_null")) & col("c3_non_null"); let expected = lit(0i64); @@ -3087,18 +3076,6 @@ mod tests { #[test] fn test_simplify_negated_bitwise_or() { - // !c4 | c4 --> -1 - let expr = (-col("c4_non_null")) | col("c4_non_null"); - let expected = lit(-1i32); - - assert_eq!(simplify(expr), expected); - - // c4 | !c4 --> -1 - let expr = col("c4_non_null") | (-col("c4_non_null")); - let expected = lit(-1i32); - - assert_eq!(simplify(expr), expected); - // !c3 | c3 --> -1 let expr = (-col("c3_non_null")) | col("c3_non_null"); let expected = lit(-1i64); @@ -3114,18 +3091,6 @@ mod tests { #[test] fn test_simplify_negated_bitwise_xor() { - // !c4 ^ c4 --> -1 - let expr = (-col("c4_non_null")) ^ col("c4_non_null"); - let expected = lit(-1i32); - - assert_eq!(simplify(expr), expected); - - // c4 ^ !c4 --> -1 - let expr = col("c4_non_null") ^ (-col("c4_non_null")); - let expected = lit(-1i32); - - assert_eq!(simplify(expr), expected); - // !c3 ^ c3 --> -1 let expr = (-col("c3_non_null")) ^ col("c3_non_null"); let expected = lit(-1i64); From b2003375f8eb458729973378b9a7f97b285d4e13 Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Thu, 16 Jul 2026 18:05:17 +0900 Subject: [PATCH 3/7] Add missing null handling arms for `arithmetic_negate` Seems like this was just a simple omission --- datafusion/common/src/scalar/mod.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/datafusion/common/src/scalar/mod.rs b/datafusion/common/src/scalar/mod.rs index 4f8a3c24eb4a5..17f0e064eb121 100644 --- a/datafusion/common/src/scalar/mod.rs +++ b/datafusion/common/src/scalar/mod.rs @@ -2303,7 +2303,18 @@ impl ScalarValue { | ScalarValue::Int64(None) | ScalarValue::Float16(None) | ScalarValue::Float32(None) - | ScalarValue::Float64(None) => Ok(self.clone()), + | ScalarValue::Float64(None) + | ScalarValue::IntervalYearMonth(None) + | ScalarValue::IntervalDayTime(None) + | ScalarValue::IntervalMonthDayNano(None) + | ScalarValue::Decimal32(None, _, _) + | ScalarValue::Decimal64(None, _, _) + | ScalarValue::Decimal128(None, _, _) + | ScalarValue::Decimal256(None, _, _) + | ScalarValue::TimestampSecond(None, _) + | ScalarValue::TimestampMillisecond(None, _) + | ScalarValue::TimestampMicrosecond(None, _) + | ScalarValue::TimestampNanosecond(None, _) => Ok(self.clone()), ScalarValue::Float16(Some(v)) => Ok(ScalarValue::Float16(Some(-v))), ScalarValue::Float64(Some(v)) => Ok(ScalarValue::Float64(Some(-v))), ScalarValue::Float32(Some(v)) => Ok(ScalarValue::Float32(Some(-v))), From c805ef03461a0875b1a985d6888b1a0454b16c49 Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Thu, 16 Jul 2026 19:21:39 +0900 Subject: [PATCH 4/7] Add upper scale bound checks for one/-one/ten for decimals For one/negative one, if the scale equals the precision then all values are to the right of the decimal point meaning we actually cannot represent a value of 1, so this could have caused some subtle errors. Similar for ten, we need at least 2 digits left of the decimal point, that is (p - s) >= 2, to represent a ten value. --- datafusion/common/src/scalar/mod.rs | 109 ++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/datafusion/common/src/scalar/mod.rs b/datafusion/common/src/scalar/mod.rs index 17f0e064eb121..f76fa3607e108 100644 --- a/datafusion/common/src/scalar/mod.rs +++ b/datafusion/common/src/scalar/mod.rs @@ -1808,6 +1808,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + *precision != *scale as u8, + "Can't represent one at scale {} with precision {}", + *scale, + *precision + ); match 10_i32.checked_pow(*scale as u32) { Some(value) => { ScalarValue::Decimal32(Some(value), *precision, *scale) @@ -1820,6 +1826,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + *precision != *scale as u8, + "Can't represent one at scale {} with precision {}", + *scale, + *precision + ); match i64::from(10).checked_pow(*scale as u32) { Some(value) => { ScalarValue::Decimal64(Some(value), *precision, *scale) @@ -1832,6 +1844,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + *precision != *scale as u8, + "Can't represent one at scale {} with precision {}", + *scale, + *precision + ); match i128::from(10).checked_pow(*scale as u32) { Some(value) => { ScalarValue::Decimal128(Some(value), *precision, *scale) @@ -1844,6 +1862,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + *precision != *scale as u8, + "Can't represent one at scale {} with precision {}", + *scale, + *precision + ); match i256::from(10).checked_pow(*scale as u32) { Some(value) => { ScalarValue::Decimal256(Some(value), *precision, *scale) @@ -1874,6 +1898,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + *precision != *scale as u8, + "Can't represent negative one at scale {} with precision {}", + *scale, + *precision + ); match 10_i32.checked_pow(*scale as u32) { Some(value) => { ScalarValue::Decimal32(Some(-value), *precision, *scale) @@ -1886,6 +1916,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + *precision != *scale as u8, + "Can't represent negative one at scale {} with precision {}", + *scale, + *precision + ); match i64::from(10).checked_pow(*scale as u32) { Some(value) => { ScalarValue::Decimal64(Some(-value), *precision, *scale) @@ -1898,6 +1934,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + *precision != *scale as u8, + "Can't represent negative one at scale {} with precision {}", + *scale, + *precision + ); match i128::from(10).checked_pow(*scale as u32) { Some(value) => { ScalarValue::Decimal128(Some(-value), *precision, *scale) @@ -1910,6 +1952,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + *precision != *scale as u8, + "Can't represent one at scale {} with precision {}", + *scale, + *precision + ); match i256::from(10).checked_pow(*scale as u32) { Some(value) => { ScalarValue::Decimal256(Some(-value), *precision, *scale) @@ -1943,6 +1991,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + (*precision - *scale as u8) > 1, + "Can't represent ten at scale {} with precision {}", + *scale, + *precision + ); match 10_i32.checked_pow((*scale + 1) as u32) { Some(value) => { ScalarValue::Decimal32(Some(value), *precision, *scale) @@ -1955,6 +2009,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + (*precision - *scale as u8) > 1, + "Can't represent ten at scale {} with precision {}", + *scale, + *precision + ); match i64::from(10).checked_pow((*scale + 1) as u32) { Some(value) => { ScalarValue::Decimal64(Some(value), *precision, *scale) @@ -1967,6 +2027,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + (*precision - *scale as u8) > 1, + "Can't represent ten at scale {} with precision {}", + *scale, + *precision + ); match i128::from(10).checked_pow((*scale + 1) as u32) { Some(value) => { ScalarValue::Decimal128(Some(value), *precision, *scale) @@ -1979,6 +2045,12 @@ impl ScalarValue { *precision, *scale, )?; assert_or_internal_err!(*scale >= 0, "Negative scale is not supported"); + assert_or_internal_err!( + (*precision - *scale as u8) > 1, + "Can't represent ten at scale {} with precision {}", + *scale, + *precision + ); match i256::from(10).checked_pow((*scale + 1) as u32) { Some(value) => { ScalarValue::Decimal256(Some(value), *precision, *scale) @@ -11472,4 +11544,41 @@ mod tests { .unwrap(); assert_eq!(s.to_string(), "[]"); } + + #[test] + fn test_decimal_value_bounds() { + fn run_tests() { + // 0.1111, 0.2222, etc. + let max_scale = D::TYPE_CONSTRUCTOR(D::MAX_PRECISION, D::MAX_SCALE); + // 1.111, 2.222, etc. + let max_scale_less_one = + D::TYPE_CONSTRUCTOR(D::MAX_PRECISION, D::MAX_SCALE - 1); + // 11.11, 22.22, etc. + let max_scale_less_two = + D::TYPE_CONSTRUCTOR(D::MAX_PRECISION, D::MAX_SCALE - 2); + + // Invalid (can't represent the value) + assert!(ScalarValue::new_one(&max_scale).is_err()); + assert!(ScalarValue::new_negative_one(&max_scale).is_err()); + assert!(ScalarValue::new_ten(&max_scale).is_err()); + assert!(ScalarValue::new_ten(&max_scale_less_one).is_err()); + + // Valid + let one = ScalarValue::Int32(Some(1)); + let neg_one = ScalarValue::Int32(Some(-1)); + let ten = ScalarValue::Int32(Some(10)); + + let num = ScalarValue::new_one(&max_scale_less_one).unwrap(); + assert_eq!(num.cast_to(&DataType::Int32).unwrap(), one); + let num = ScalarValue::new_negative_one(&max_scale_less_one).unwrap(); + assert_eq!(num.cast_to(&DataType::Int32).unwrap(), neg_one); + let num = ScalarValue::new_ten(&max_scale_less_two).unwrap(); + assert_eq!(num.cast_to(&DataType::Int32).unwrap(), ten); + } + + run_tests::(); + run_tests::(); + run_tests::(); + run_tests::(); + } } From da7a17a621a1958189318952b33e35323688e16b Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Thu, 16 Jul 2026 19:22:51 +0900 Subject: [PATCH 5/7] Retrieve one/-one/ten values from precomputed constants for decimals Using macros here to generate these tables instead of manually enumerating the values into the code itself; for i256 its a bit complicated since it doesn't have native support so can't use direct multiplication as it doesn't have const support. Used Codex in this case, but added a unit test to ensure the constant values generated are correct. --- datafusion/common/src/scalar/consts.rs | 60 ++++++++++++++ datafusion/common/src/scalar/mod.rs | 107 ++++++++----------------- 2 files changed, 95 insertions(+), 72 deletions(-) diff --git a/datafusion/common/src/scalar/consts.rs b/datafusion/common/src/scalar/consts.rs index 599c2523cd2c7..9991ad3e213dd 100644 --- a/datafusion/common/src/scalar/consts.rs +++ b/datafusion/common/src/scalar/consts.rs @@ -17,6 +17,9 @@ // Constants defined for scalar construction. +use arrow::datatypes::{Decimal32Type, Decimal64Type, Decimal128Type, DecimalType}; +use arrow::datatypes::{Decimal256Type, i256}; + // Next F16 value above π (upper bound) pub(super) const PI_UPPER_F16: half::f16 = half::f16::from_bits(0x4249); @@ -54,3 +57,60 @@ pub(super) const NEGATIVE_FRAC_PI_2_LOWER_F32: f32 = // Next f64 value below -π/2 (lower bound) pub(super) const NEGATIVE_FRAC_PI_2_LOWER_F64: f64 = (-std::f64::consts::FRAC_PI_2).next_down(); + +// Generate lookup table for 1 values of decimals (1, 10, 100, etc.) +macro_rules! decimal_ones_lut { + () => {{ + let mut values = [1; _]; + let mut i = 1; + while i < values.len() { + values[i] = values[i - 1] * 10; + i += 1; + } + values + }}; +} + +// 1, 10, 100 values meant to be indexed by scale. We omit handling for MAX_SCALE +// itself (we don't go to MAX_SCALE + 1) since we can't represent a 1 value at +// that scale. +pub(super) const DECIMAL32_ONES: [i32; Decimal32Type::MAX_SCALE as usize] = + decimal_ones_lut!(); +pub(super) const DECIMAL64_ONES: [i64; Decimal64Type::MAX_SCALE as usize] = + decimal_ones_lut!(); +pub(super) const DECIMAL128_ONES: [i128; Decimal128Type::MAX_SCALE as usize] = + decimal_ones_lut!(); +pub(super) const DECIMAL256_ONES: [i256; Decimal256Type::MAX_SCALE as usize] = { + // This code was generated by codex and frankly I don't know how it works, + // but the test below verifies it outputs the correct values so ¯\_(ツ)_/¯ + // + // This is mainly a shortcut for not needing to manually list out each value + // anyway. + let mut values = [i256::ONE; _]; + let mut i = 1; + while i < values.len() { + let (low, high) = values[i - 1].to_parts(); + let low_product = (low as u64 as u128) * 10; + let high_product = (low >> 64) * 10 + (low_product >> 64); + let low = ((high_product as u64 as u128) << 64) | low_product as u64 as u128; + let carry = (high_product >> 64) as i128; + values[i] = i256::from_parts(low, high * 10 + carry); + i += 1; + } + values +}; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_ensure_correct_decimal256_ones() { + for (scale, val) in DECIMAL256_ONES.iter().enumerate() { + let zeros = "0".repeat(scale); + let num = "1".to_string() + &zeros; + let num = i256::from_string(&num).unwrap(); + assert_eq!(num, *val, "{scale}"); + } + } +} diff --git a/datafusion/common/src/scalar/mod.rs b/datafusion/common/src/scalar/mod.rs index f76fa3607e108..924620a930869 100644 --- a/datafusion/common/src/scalar/mod.rs +++ b/datafusion/common/src/scalar/mod.rs @@ -54,6 +54,9 @@ use crate::cast::{ use crate::error::{_exec_err, _internal_err, _not_impl_err, DataFusionError, Result}; use crate::format::DEFAULT_CAST_OPTIONS; use crate::hash_utils::create_hashes; +use crate::scalar::consts::{ + DECIMAL32_ONES, DECIMAL64_ONES, DECIMAL128_ONES, DECIMAL256_ONES, +}; use crate::utils::SingleRowListArrayBuilder; use crate::{_internal_datafusion_err, arrow_datafusion_err}; use arrow::array::{ @@ -1814,12 +1817,8 @@ impl ScalarValue { *scale, *precision ); - match 10_i32.checked_pow(*scale as u32) { - Some(value) => { - ScalarValue::Decimal32(Some(value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + let one = DECIMAL32_ONES[*scale as usize]; + ScalarValue::Decimal32(Some(one), *precision, *scale) } DataType::Decimal64(precision, scale) => { Self::validate_decimal_or_internal_err::( @@ -1832,12 +1831,8 @@ impl ScalarValue { *scale, *precision ); - match i64::from(10).checked_pow(*scale as u32) { - Some(value) => { - ScalarValue::Decimal64(Some(value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + let one = DECIMAL64_ONES[*scale as usize]; + ScalarValue::Decimal64(Some(one), *precision, *scale) } DataType::Decimal128(precision, scale) => { Self::validate_decimal_or_internal_err::( @@ -1850,12 +1845,8 @@ impl ScalarValue { *scale, *precision ); - match i128::from(10).checked_pow(*scale as u32) { - Some(value) => { - ScalarValue::Decimal128(Some(value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + let one = DECIMAL128_ONES[*scale as usize]; + ScalarValue::Decimal128(Some(one), *precision, *scale) } DataType::Decimal256(precision, scale) => { Self::validate_decimal_or_internal_err::( @@ -1868,12 +1859,8 @@ impl ScalarValue { *scale, *precision ); - match i256::from(10).checked_pow(*scale as u32) { - Some(value) => { - ScalarValue::Decimal256(Some(value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + let one = DECIMAL256_ONES[*scale as usize]; + ScalarValue::Decimal256(Some(one), *precision, *scale) } _ => { return _not_impl_err!( @@ -1904,12 +1891,8 @@ impl ScalarValue { *scale, *precision ); - match 10_i32.checked_pow(*scale as u32) { - Some(value) => { - ScalarValue::Decimal32(Some(-value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + let one = DECIMAL32_ONES[*scale as usize]; + ScalarValue::Decimal32(Some(-one), *precision, *scale) } DataType::Decimal64(precision, scale) => { Self::validate_decimal_or_internal_err::( @@ -1922,12 +1905,8 @@ impl ScalarValue { *scale, *precision ); - match i64::from(10).checked_pow(*scale as u32) { - Some(value) => { - ScalarValue::Decimal64(Some(-value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + let one = DECIMAL64_ONES[*scale as usize]; + ScalarValue::Decimal64(Some(-one), *precision, *scale) } DataType::Decimal128(precision, scale) => { Self::validate_decimal_or_internal_err::( @@ -1940,12 +1919,8 @@ impl ScalarValue { *scale, *precision ); - match i128::from(10).checked_pow(*scale as u32) { - Some(value) => { - ScalarValue::Decimal128(Some(-value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + let one = DECIMAL128_ONES[*scale as usize]; + ScalarValue::Decimal128(Some(-one), *precision, *scale) } DataType::Decimal256(precision, scale) => { Self::validate_decimal_or_internal_err::( @@ -1958,12 +1933,8 @@ impl ScalarValue { *scale, *precision ); - match i256::from(10).checked_pow(*scale as u32) { - Some(value) => { - ScalarValue::Decimal256(Some(-value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + let one = DECIMAL256_ONES[*scale as usize]; + ScalarValue::Decimal256(Some(-one), *precision, *scale) } _ => { return _not_impl_err!( @@ -1997,12 +1968,10 @@ impl ScalarValue { *scale, *precision ); - match 10_i32.checked_pow((*scale + 1) as u32) { - Some(value) => { - ScalarValue::Decimal32(Some(value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + // +1 safe since we validate above that scale must be less than + // the max possible scale + let ten = DECIMAL32_ONES[*scale as usize + 1]; + ScalarValue::Decimal32(Some(ten), *precision, *scale) } DataType::Decimal64(precision, scale) => { Self::validate_decimal_or_internal_err::( @@ -2015,12 +1984,10 @@ impl ScalarValue { *scale, *precision ); - match i64::from(10).checked_pow((*scale + 1) as u32) { - Some(value) => { - ScalarValue::Decimal64(Some(value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + // +1 safe since we validate above that scale must be less than + // the max possible scale + let ten = DECIMAL64_ONES[*scale as usize + 1]; + ScalarValue::Decimal64(Some(ten), *precision, *scale) } DataType::Decimal128(precision, scale) => { Self::validate_decimal_or_internal_err::( @@ -2033,12 +2000,10 @@ impl ScalarValue { *scale, *precision ); - match i128::from(10).checked_pow((*scale + 1) as u32) { - Some(value) => { - ScalarValue::Decimal128(Some(value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + // +1 safe since we validate above that scale must be less than + // the max possible scale + let ten = DECIMAL128_ONES[*scale as usize + 1]; + ScalarValue::Decimal128(Some(ten), *precision, *scale) } DataType::Decimal256(precision, scale) => { Self::validate_decimal_or_internal_err::( @@ -2051,12 +2016,10 @@ impl ScalarValue { *scale, *precision ); - match i256::from(10).checked_pow((*scale + 1) as u32) { - Some(value) => { - ScalarValue::Decimal256(Some(value), *precision, *scale) - } - None => return _internal_err!("Unsupported scale {scale}"), - } + // +1 safe since we validate above that scale must be less than + // the max possible scale + let ten = DECIMAL256_ONES[*scale as usize + 1]; + ScalarValue::Decimal256(Some(ten), *precision, *scale) } _ => { return _not_impl_err!( From 3108c0adacbbed2aa148eb89ddd51d172760d6dc Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Thu, 16 Jul 2026 19:25:45 +0900 Subject: [PATCH 6/7] Make use of `ScalarValue` code in simplify expressions utils I noticed we didn't have support for `Float16`, `Decimal32` and `Decimal64` here; instead of just adding them here, I figured it would be easier to plumb in the `ScalarValue` code to reduce duplication and ensure there's less opportunities for subtle omissions like this. --- .../src/simplify_expressions/utils.rs | 102 +++--------------- 1 file changed, 16 insertions(+), 86 deletions(-) diff --git a/datafusion/optimizer/src/simplify_expressions/utils.rs b/datafusion/optimizer/src/simplify_expressions/utils.rs index b0908b47602f7..89bb762d59ce2 100644 --- a/datafusion/optimizer/src/simplify_expressions/utils.rs +++ b/datafusion/optimizer/src/simplify_expressions/utils.rs @@ -17,7 +17,6 @@ //! Utility functions for expression simplification -use arrow::datatypes::i256; use datafusion_common::{Result, ScalarValue, internal_err}; use datafusion_expr::{ Case, Expr, Like, Operator, @@ -25,47 +24,6 @@ use datafusion_expr::{ expr_fn::{and, bitwise_and, bitwise_or, or}, }; -pub static POWS_OF_TEN: [i128; 38] = [ - 1, - 10, - 100, - 1000, - 10000, - 100000, - 1000000, - 10000000, - 100000000, - 1000000000, - 10000000000, - 100000000000, - 1000000000000, - 10000000000000, - 100000000000000, - 1000000000000000, - 10000000000000000, - 100000000000000000, - 1000000000000000000, - 10000000000000000000, - 100000000000000000000, - 1000000000000000000000, - 10000000000000000000000, - 100000000000000000000000, - 1000000000000000000000000, - 10000000000000000000000000, - 100000000000000000000000000, - 1000000000000000000000000000, - 10000000000000000000000000000, - 100000000000000000000000000000, - 1000000000000000000000000000000, - 10000000000000000000000000000000, - 100000000000000000000000000000000, - 1000000000000000000000000000000000, - 10000000000000000000000000000000000, - 100000000000000000000000000000000000, - 1000000000000000000000000000000000000, - 10000000000000000000000000000000000000, -]; - /// returns true if `needle` is found in a chain of search_op /// expressions. Such as: (A AND B) AND C fn expr_contains_inner(expr: &Expr, needle: &Expr, search_op: Operator) -> bool { @@ -139,54 +97,26 @@ pub fn delete_xor_in_complex_expr(expr: &Expr, needle: &Expr, is_left: bool) -> } pub fn is_zero(s: &Expr) -> bool { - match s { - Expr::Literal(ScalarValue::Int8(Some(0)), _) - | Expr::Literal(ScalarValue::Int16(Some(0)), _) - | Expr::Literal(ScalarValue::Int32(Some(0)), _) - | Expr::Literal(ScalarValue::Int64(Some(0)), _) - | Expr::Literal(ScalarValue::UInt8(Some(0)), _) - | Expr::Literal(ScalarValue::UInt16(Some(0)), _) - | Expr::Literal(ScalarValue::UInt32(Some(0)), _) - | Expr::Literal(ScalarValue::UInt64(Some(0)), _) => true, - Expr::Literal(ScalarValue::Float32(Some(v)), _) if *v == 0. => true, - Expr::Literal(ScalarValue::Float64(Some(v)), _) if *v == 0. => true, - Expr::Literal(ScalarValue::Decimal128(Some(v), _p, _s), _) if *v == 0 => true, - Expr::Literal(ScalarValue::Decimal256(Some(v), _p, _s), _) - if *v == i256::ZERO => - { - true - } - _ => false, + if let Expr::Literal(sv, _) = s + && sv.data_type().is_numeric() + { + // unwrap safe since numeric types always have a 0 value + sv == &ScalarValue::new_zero(&sv.data_type()).unwrap() + } else { + false } } pub fn is_one(s: &Expr) -> bool { - match s { - Expr::Literal(ScalarValue::Int8(Some(1)), _) - | Expr::Literal(ScalarValue::Int16(Some(1)), _) - | Expr::Literal(ScalarValue::Int32(Some(1)), _) - | Expr::Literal(ScalarValue::Int64(Some(1)), _) - | Expr::Literal(ScalarValue::UInt8(Some(1)), _) - | Expr::Literal(ScalarValue::UInt16(Some(1)), _) - | Expr::Literal(ScalarValue::UInt32(Some(1)), _) - | Expr::Literal(ScalarValue::UInt64(Some(1)), _) => true, - Expr::Literal(ScalarValue::Float32(Some(v)), _) if *v == 1. => true, - Expr::Literal(ScalarValue::Float64(Some(v)), _) if *v == 1. => true, - Expr::Literal(ScalarValue::Decimal128(Some(v), _p, s), _) => { - *s >= 0 - && POWS_OF_TEN - .get(*s as usize) - .map(|x| x == v) - .unwrap_or_default() - } - Expr::Literal(ScalarValue::Decimal256(Some(v), _p, s), _) => { - *s >= 0 - && match i256::from(10).checked_pow(*s as u32) { - Some(res) => res == *v, - None => false, - } - } - _ => false, + if let Expr::Literal(sv, _) = s + && sv.data_type().is_numeric() + // there are edge cases like negative scale decimals not being able to + // create a one value so this can fail + && let Ok(one) = ScalarValue::new_one(&sv.data_type()) + { + sv == &one + } else { + false } } From ce0f16ba314008d04ee9c77ecf44a85e0a24edf7 Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Fri, 17 Jul 2026 22:18:29 +0900 Subject: [PATCH 7/7] add TODO --- datafusion/common/src/scalar/consts.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/datafusion/common/src/scalar/consts.rs b/datafusion/common/src/scalar/consts.rs index 9991ad3e213dd..df12265a3723c 100644 --- a/datafusion/common/src/scalar/consts.rs +++ b/datafusion/common/src/scalar/consts.rs @@ -86,6 +86,9 @@ pub(super) const DECIMAL256_ONES: [i256; Decimal256Type::MAX_SCALE as usize] = { // // This is mainly a shortcut for not needing to manually list out each value // anyway. + // + // TODO: simplify this after https://github.com/apache/arrow-rs/pull/10363 + // lands upstream let mut values = [i256::ONE; _]; let mut i = 1; while i < values.len() {