Skip to content

Commit 0fc55d0

Browse files
authored
refactor: make scalar distance u64 and overflow aware (#22892)
Closes #22687 ## Rationale for this change The distance API in `datafusion/common/src/scalar/mod.rs` previously returned `Option<usize>`. `usize` is machine-width dependent and does not represent value-domain cardinality. This could lead to target-dependent behavior on large integer/temporal ranges. Additionally, downstream callers like `interval_arithmetic.rs` had to convert the distance back to `u64` to compute cardinality. Exposing an overflow-aware `u64`-oriented contract (`distance_u64`) resolves these architecture differences and aligns the API with value-domain semantics. ## What changes are included in this PR? - Added `distance_u64`: Added a new public method `distance_u64(&self, other: &ScalarValue) -> Option<u64>` to `ScalarValue`. - Deprecated `distance`: Marked the original `distance(&self, other: &ScalarValue) -> Option<usize>` method as deprecated and redirected it to call `distance_u64`. - Interval Cardinality: Migrated the cardinality calculation in `datafusion/expr-common/src/interval_arithmetic.rs` to use `distance_u64` directly. - Selectivity / Stats Overlap: Migrated the overlap calculations in `datafusion/common/src/stats.rs` to use `distance_u64`. - Boundary/Overflow Tests: Added `test_scalar_distance_u64_boundaries` in `scalar/mod.rs` to verify edge cases: - Full signed range edge (`i64::MIN` to `i64::MAX`) - Full unsigned range edge (`u64::MIN` to `u64::MAX`) - Large temporal range edge (`TimestampSecond` and `Date32` boundaries) - Overflow-to-None behavior (exceeding `u64::MAX` for Float, `Decimal128`, and `Decimal256` values) ## Are these changes tested? Yes, they are covered by the new unit tests in `datafusion-common` and existing test suites in both `datafusion-common` and `datafusion-expr-common`. ## Are there any user-facing changes? No, deprecation of `ScalarValue::distance` has been removed from this PR
1 parent 1f45d83 commit 0fc55d0

5 files changed

Lines changed: 209 additions & 40 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

datafusion/common/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ indexmap = { workspace = true }
7777
itertools = { workspace = true }
7878
libc = "0.2.185"
7979
log = { workspace = true }
80+
num-traits = { workspace = true }
8081
object_store = { workspace = true, optional = true }
8182
parquet = { workspace = true, optional = true, default-features = true }
8283
recursive = { workspace = true, optional = true }

datafusion/common/src/scalar/mod.rs

Lines changed: 189 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ use arrow::util::display::{ArrayFormatter, FormatOptions, array_value_to_string}
9292
use cache::{get_or_create_cached_key_array, get_or_create_cached_null_array};
9393
use chrono::{Duration, NaiveDate};
9494
use half::f16;
95+
use num_traits::ToPrimitive;
9596
pub use struct_builder::ScalarStructBuilder;
9697

9798
const SECONDS_PER_DAY: i64 = 86_400;
@@ -2585,63 +2586,107 @@ impl ScalarValue {
25852586
/// distance is greater than [`usize::MAX`]. If the type is a float, then the distance will be
25862587
/// rounded to the nearest integer.
25872588
///
2588-
///
25892589
/// Note: the datatype itself must support subtraction.
25902590
pub fn distance(&self, other: &ScalarValue) -> Option<usize> {
2591+
self.distance_u64(other)
2592+
.and_then(|d| usize::try_from(d).ok())
2593+
}
2594+
2595+
/// Helper to convert a rounded float distance to u64, returning None if it exceeds u64::MAX, is negative, or is not finite.
2596+
fn rounded_float_distance_u64(diff: f64) -> Option<u64> {
2597+
if diff.is_finite() && diff >= 0.0 && diff < u64::MAX as f64 {
2598+
Some(diff as u64)
2599+
} else {
2600+
None
2601+
}
2602+
}
2603+
2604+
/// Absolute distance between two numeric values (of the same type). This method will return
2605+
/// None if either one of the arguments are null. It might also return None if the resulting
2606+
/// distance is greater than [`u64::MAX`]. If the type is a float, then the distance will be
2607+
/// rounded to the nearest integer.
2608+
///
2609+
/// Note: the datatype itself must support subtraction.
2610+
pub fn distance_u64(&self, other: &ScalarValue) -> Option<u64> {
25912611
match (self, other) {
2592-
(Self::Int8(Some(l)), Self::Int8(Some(r))) => Some(l.abs_diff(*r) as _),
2593-
(Self::Int16(Some(l)), Self::Int16(Some(r))) => Some(l.abs_diff(*r) as _),
2594-
(Self::Int32(Some(l)), Self::Int32(Some(r))) => Some(l.abs_diff(*r) as _),
2595-
(Self::Int64(Some(l)), Self::Int64(Some(r))) => Some(l.abs_diff(*r) as _),
2596-
(Self::UInt8(Some(l)), Self::UInt8(Some(r))) => Some(l.abs_diff(*r) as _),
2597-
(Self::UInt16(Some(l)), Self::UInt16(Some(r))) => Some(l.abs_diff(*r) as _),
2598-
(Self::UInt32(Some(l)), Self::UInt32(Some(r))) => Some(l.abs_diff(*r) as _),
2599-
(Self::UInt64(Some(l)), Self::UInt64(Some(r))) => Some(l.abs_diff(*r) as _),
2612+
(Self::Int8(Some(l)), Self::Int8(Some(r))) => Some(l.abs_diff(*r) as u64),
2613+
(Self::Int16(Some(l)), Self::Int16(Some(r))) => Some(l.abs_diff(*r) as u64),
2614+
(Self::Int32(Some(l)), Self::Int32(Some(r))) => Some(l.abs_diff(*r) as u64),
2615+
(Self::Int64(Some(l)), Self::Int64(Some(r))) => Some(l.abs_diff(*r)),
2616+
(Self::UInt8(Some(l)), Self::UInt8(Some(r))) => Some(l.abs_diff(*r) as u64),
2617+
(Self::UInt16(Some(l)), Self::UInt16(Some(r))) => Some(l.abs_diff(*r) as u64),
2618+
(Self::UInt32(Some(l)), Self::UInt32(Some(r))) => Some(l.abs_diff(*r) as u64),
2619+
(Self::UInt64(Some(l)), Self::UInt64(Some(r))) => Some(l.abs_diff(*r)),
26002620
// TODO: we might want to look into supporting ceil/floor here for floats.
26012621
(Self::Float16(Some(l)), Self::Float16(Some(r))) => {
2602-
Some((f16::to_f32(*l) - f16::to_f32(*r)).abs().round() as _)
2622+
let diff = (f16::to_f32(*l) - f16::to_f32(*r)).abs().round();
2623+
Self::rounded_float_distance_u64(diff as f64)
26032624
}
26042625
(Self::Float32(Some(l)), Self::Float32(Some(r))) => {
2605-
Some((l - r).abs().round() as _)
2626+
let diff = (l - r).abs().round();
2627+
Self::rounded_float_distance_u64(diff as f64)
26062628
}
26072629
(Self::Float64(Some(l)), Self::Float64(Some(r))) => {
2608-
Some((l - r).abs().round() as _)
2630+
let diff = (l - r).abs().round();
2631+
Self::rounded_float_distance_u64(diff)
26092632
}
2610-
(Self::Date32(Some(l)), Self::Date32(Some(r))) => Some(l.abs_diff(*r) as _),
2611-
(Self::Date64(Some(l)), Self::Date64(Some(r))) => Some(l.abs_diff(*r) as _),
2633+
(Self::Date32(Some(l)), Self::Date32(Some(r))) => Some(l.abs_diff(*r) as u64),
2634+
(Self::Date64(Some(l)), Self::Date64(Some(r))) => Some(l.abs_diff(*r)),
26122635
// Timestamp values are stored as epoch ticks regardless of timezone
26132636
// annotation, so the distance is tz-independent (tz is display metadata).
26142637
(Self::TimestampSecond(Some(l), _), Self::TimestampSecond(Some(r), _)) => {
2615-
Some(l.abs_diff(*r) as _)
2638+
Some(l.abs_diff(*r))
26162639
}
26172640
(
26182641
Self::TimestampMillisecond(Some(l), _),
26192642
Self::TimestampMillisecond(Some(r), _),
2620-
) => Some(l.abs_diff(*r) as _),
2643+
) => Some(l.abs_diff(*r)),
26212644
(
26222645
Self::TimestampMicrosecond(Some(l), _),
26232646
Self::TimestampMicrosecond(Some(r), _),
2624-
) => Some(l.abs_diff(*r) as _),
2647+
) => Some(l.abs_diff(*r)),
26252648
(
26262649
Self::TimestampNanosecond(Some(l), _),
26272650
Self::TimestampNanosecond(Some(r), _),
2628-
) => Some(l.abs_diff(*r) as _),
2651+
) => Some(l.abs_diff(*r)),
2652+
(
2653+
Self::Decimal32(Some(l), _, lscale),
2654+
Self::Decimal32(Some(r), _, rscale),
2655+
) => {
2656+
// In order to be aligned with PartialOrd we only
2657+
// check for equal scale, ignoring precision
2658+
if lscale == rscale {
2659+
Some(l.abs_diff(*r) as u64)
2660+
} else {
2661+
None
2662+
}
2663+
}
2664+
(
2665+
Self::Decimal64(Some(l), _, lscale),
2666+
Self::Decimal64(Some(r), _, rscale),
2667+
) => {
2668+
if lscale == rscale {
2669+
Some(l.abs_diff(*r))
2670+
} else {
2671+
None
2672+
}
2673+
}
26292674
(
2630-
Self::Decimal128(Some(l), lprecision, lscale),
2631-
Self::Decimal128(Some(r), rprecision, rscale),
2675+
Self::Decimal128(Some(l), _, lscale),
2676+
Self::Decimal128(Some(r), _, rscale),
26322677
) => {
2633-
if lprecision == rprecision && lscale == rscale {
2634-
l.checked_sub(*r)?.checked_abs()?.to_usize()
2678+
if lscale == rscale {
2679+
l.checked_sub(*r)?.checked_abs()?.to_u64()
26352680
} else {
26362681
None
26372682
}
26382683
}
26392684
(
2640-
Self::Decimal256(Some(l), lprecision, lscale),
2641-
Self::Decimal256(Some(r), rprecision, rscale),
2685+
Self::Decimal256(Some(l), _, lscale),
2686+
Self::Decimal256(Some(r), _, rscale),
26422687
) => {
2643-
if lprecision == rprecision && lscale == rscale {
2644-
l.checked_sub(*r)?.checked_abs()?.to_usize()
2688+
if lscale == rscale {
2689+
l.checked_sub(*r)?.checked_abs()?.to_u64()
26452690
} else {
26462691
None
26472692
}
@@ -9444,8 +9489,8 @@ mod tests {
94449489
),
94459490
];
94469491
for (lhs, rhs, expected) in cases.iter() {
9447-
let distance = lhs.distance(rhs).unwrap();
9448-
assert_eq!(distance, *expected);
9492+
let distance = lhs.distance_u64(rhs).unwrap();
9493+
assert_eq!(distance, *expected as u64);
94499494
}
94509495
}
94519496

@@ -9462,7 +9507,7 @@ mod tests {
94629507
),
94639508
];
94649509
for (lhs, rhs) in cases.iter() {
9465-
let distance = lhs.distance(rhs);
9510+
let distance = lhs.distance_u64(rhs);
94669511
assert!(distance.is_none(), "{lhs} vs {rhs}");
94679512
}
94689513
}
@@ -9508,13 +9553,9 @@ mod tests {
95089553
ScalarValue::Decimal128(Some(123), 5, 5),
95099554
ScalarValue::Decimal128(Some(120), 5, 3),
95109555
),
9511-
(
9512-
ScalarValue::Decimal128(Some(123), 5, 5),
9513-
ScalarValue::Decimal128(Some(120), 3, 5),
9514-
),
95159556
(
95169557
ScalarValue::Decimal256(Some(123.into()), 5, 5),
9517-
ScalarValue::Decimal256(Some(120.into()), 3, 5),
9558+
ScalarValue::Decimal256(Some(120.into()), 5, 3),
95189559
),
95199560
// Distance 2 * 2^50 is larger than usize
95209561
(
@@ -9536,11 +9577,124 @@ mod tests {
95369577
),
95379578
];
95389579
for (lhs, rhs) in cases {
9539-
let distance = lhs.distance(&rhs);
9580+
let distance = lhs.distance_u64(&rhs);
95409581
assert!(distance.is_none());
95419582
}
95429583
}
95439584

9585+
#[test]
9586+
fn test_scalar_distance_u64_boundaries() {
9587+
// 1. Full-domain integer ranges
9588+
// i64::MIN to i64::MAX -> distance is u64::MAX
9589+
let lhs = ScalarValue::Int64(Some(i64::MIN));
9590+
let rhs = ScalarValue::Int64(Some(i64::MAX));
9591+
assert_eq!(lhs.distance_u64(&rhs), Some(u64::MAX));
9592+
assert_eq!(rhs.distance_u64(&lhs), Some(u64::MAX));
9593+
9594+
// u64::MIN to u64::MAX -> distance is u64::MAX
9595+
let lhs = ScalarValue::UInt64(Some(u64::MIN));
9596+
let rhs = ScalarValue::UInt64(Some(u64::MAX));
9597+
assert_eq!(lhs.distance_u64(&rhs), Some(u64::MAX));
9598+
assert_eq!(rhs.distance_u64(&lhs), Some(u64::MAX));
9599+
9600+
// 2. Decimal128 overflow edges (around u64::MAX)
9601+
// distance equal to u64::MAX fits
9602+
let lhs = ScalarValue::Decimal128(Some(0), 20, 0);
9603+
let rhs = ScalarValue::Decimal128(Some(u64::MAX as i128), 20, 0);
9604+
assert_eq!(lhs.distance_u64(&rhs), Some(u64::MAX));
9605+
9606+
// distance greater than u64::MAX overflows
9607+
let lhs = ScalarValue::Decimal128(Some(0), 20, 0);
9608+
let rhs = ScalarValue::Decimal128(Some(u64::MAX as i128 + 1), 20, 0);
9609+
assert_eq!(lhs.distance_u64(&rhs), None);
9610+
9611+
// 3. Decimal256 overflow edges (around u64::MAX)
9612+
// distance equal to u64::MAX fits
9613+
let lhs = ScalarValue::Decimal256(Some(i256::from_parts(0, 0)), 20, 0);
9614+
let rhs =
9615+
ScalarValue::Decimal256(Some(i256::from_parts(u64::MAX as u128, 0)), 20, 0);
9616+
assert_eq!(lhs.distance_u64(&rhs), Some(u64::MAX));
9617+
9618+
// distance greater than u64::MAX overflows
9619+
let lhs = ScalarValue::Decimal256(Some(i256::from_parts(0, 0)), 20, 0);
9620+
let rhs = ScalarValue::Decimal256(
9621+
Some(i256::from_parts(u64::MAX as u128 + 1, 0)),
9622+
20,
9623+
0,
9624+
);
9625+
assert_eq!(lhs.distance_u64(&rhs), None);
9626+
9627+
// 4. Float64 overflow edges (around u64::MAX)
9628+
let lhs = ScalarValue::Float64(Some(0.0));
9629+
let val: f64 = 18446744073709500000.0;
9630+
let rhs = ScalarValue::Float64(Some(val));
9631+
assert_eq!(lhs.distance_u64(&rhs), Some(18446744073709500416));
9632+
9633+
// float value > u64::MAX overflows
9634+
let rhs = ScalarValue::Float64(Some(1.9e19));
9635+
assert_eq!(lhs.distance_u64(&rhs), None);
9636+
9637+
// exact 2^64 boundary (18446744073709551616.0) is greater than u64::MAX, so it should return None
9638+
let exact_2_64_f64 = ScalarValue::Float64(Some(18446744073709551616.0));
9639+
assert_eq!(lhs.distance_u64(&exact_2_64_f64), None);
9640+
9641+
// exact 2^64 boundary as Float32 should also return None
9642+
let lhs_f32 = ScalarValue::Float32(Some(0.0));
9643+
let exact_2_64_f32 = ScalarValue::Float32(Some(18446744073709551616.0));
9644+
assert_eq!(lhs_f32.distance_u64(&exact_2_64_f32), None);
9645+
9646+
// largest float32 value below 2^64 (2^64 - 2^41 = 18446741874686296064.0) should fit
9647+
let below_2_64_f32 = ScalarValue::Float32(Some(18446741874686296064.0));
9648+
assert_eq!(
9649+
lhs_f32.distance_u64(&below_2_64_f32),
9650+
Some(18446741874686296064)
9651+
);
9652+
9653+
// Inf, NegInf, NaN
9654+
let inf = ScalarValue::Float64(Some(f64::INFINITY));
9655+
let neg_inf = ScalarValue::Float64(Some(f64::NEG_INFINITY));
9656+
let nan = ScalarValue::Float64(Some(f64::NAN));
9657+
assert_eq!(lhs.distance_u64(&inf), None);
9658+
assert_eq!(lhs.distance_u64(&neg_inf), None);
9659+
assert_eq!(lhs.distance_u64(&nan), None);
9660+
9661+
let inf_f32 = ScalarValue::Float32(Some(f32::INFINITY));
9662+
let neg_inf_f32 = ScalarValue::Float32(Some(f32::NEG_INFINITY));
9663+
let nan_f32 = ScalarValue::Float32(Some(f32::NAN));
9664+
assert_eq!(lhs_f32.distance_u64(&inf_f32), None);
9665+
assert_eq!(lhs_f32.distance_u64(&neg_inf_f32), None);
9666+
assert_eq!(lhs_f32.distance_u64(&nan_f32), None);
9667+
9668+
let lhs_f16 = ScalarValue::Float16(Some(f16::ZERO));
9669+
let inf_f16 = ScalarValue::Float16(Some(f16::INFINITY));
9670+
let neg_inf_f16 = ScalarValue::Float16(Some(f16::NEG_INFINITY));
9671+
let nan_f16 = ScalarValue::Float16(Some(f16::NAN));
9672+
assert_eq!(lhs_f16.distance_u64(&inf_f16), None);
9673+
assert_eq!(lhs_f16.distance_u64(&neg_inf_f16), None);
9674+
assert_eq!(lhs_f16.distance_u64(&nan_f16), None);
9675+
9676+
// 5. Date and Timestamp boundaries
9677+
// Date32: i32::MIN to i32::MAX
9678+
let lhs = ScalarValue::Date32(Some(i32::MIN));
9679+
let rhs = ScalarValue::Date32(Some(i32::MAX));
9680+
assert_eq!(lhs.distance_u64(&rhs), Some(u32::MAX as u64));
9681+
9682+
// TimestampSecond: i64::MIN to i64::MAX
9683+
let lhs = ScalarValue::TimestampSecond(Some(i64::MIN), None);
9684+
let rhs = ScalarValue::TimestampSecond(Some(i64::MAX), None);
9685+
assert_eq!(lhs.distance_u64(&rhs), Some(u64::MAX));
9686+
9687+
// 6. Decimal scale matching (ignoring precision)
9688+
let lhs = ScalarValue::Decimal128(Some(100), 10, 2);
9689+
let rhs = ScalarValue::Decimal128(Some(150), 15, 2);
9690+
assert_eq!(lhs.distance_u64(&rhs), Some(50));
9691+
assert_eq!(rhs.distance_u64(&lhs), Some(50));
9692+
9693+
let lhs = ScalarValue::Decimal128(Some(100), 10, 2);
9694+
let rhs = ScalarValue::Decimal128(Some(150), 10, 3);
9695+
assert_eq!(lhs.distance_u64(&rhs), None);
9696+
}
9697+
95449698
#[test]
95459699
fn test_scalar_interval_negate() {
95469700
let cases = [

datafusion/common/src/stats.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -829,8 +829,8 @@ pub fn estimate_ndv_with_overlap(
829829
let right_min = right.min_value.get_value()?;
830830
let right_max = right.max_value.get_value()?;
831831

832-
let range_left = left_max.distance(left_min)?;
833-
let range_right = right_max.distance(right_min)?;
832+
let range_left = left_max.distance_u64(left_min)?;
833+
let range_right = right_max.distance_u64(right_min)?;
834834

835835
// Constant columns (range == 0) can't use the proportional overlap
836836
// formula below, so check interval overlap directly instead.
@@ -859,7 +859,7 @@ pub fn estimate_ndv_with_overlap(
859859
return Some(ndv_left + ndv_right);
860860
}
861861

862-
let overlap_range = overlap_max.distance(overlap_min)? as f64;
862+
let overlap_range = overlap_max.distance_u64(overlap_min)? as f64;
863863

864864
let overlap_left = overlap_range / range_left as f64;
865865
let overlap_right = overlap_range / range_right as f64;

datafusion/expr-common/src/interval_arithmetic.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -910,10 +910,16 @@ impl Interval {
910910
if data_type.is_integer()
911911
|| matches!(
912912
data_type,
913-
DataType::Date32 | DataType::Date64 | DataType::Timestamp(_, _)
913+
DataType::Date32
914+
| DataType::Date64
915+
| DataType::Timestamp(_, _)
916+
| DataType::Decimal32(_, _)
917+
| DataType::Decimal64(_, _)
918+
| DataType::Decimal128(_, _)
919+
| DataType::Decimal256(_, _)
914920
)
915921
{
916-
self.upper.distance(&self.lower).map(|diff| diff as u64)
922+
self.upper.distance_u64(&self.lower)
917923
} else if data_type.is_floating() {
918924
// Negative numbers are sorted in the reverse order. To
919925
// always have a positive difference after the subtraction,
@@ -4157,6 +4163,13 @@ mod tests {
41574163
ScalarValue::TimestampNanosecond(Some(2_000_000_000), None),
41584164
)?;
41594165
assert_eq!(interval.cardinality().unwrap(), 1_000_000_001);
4166+
4167+
// Decimal types
4168+
let interval = Interval::try_new(
4169+
ScalarValue::Decimal128(Some(100), 10, 2),
4170+
ScalarValue::Decimal128(Some(110), 10, 2),
4171+
)?;
4172+
assert_eq!(interval.cardinality().unwrap(), 11);
41604173
Ok(())
41614174
}
41624175

0 commit comments

Comments
 (0)