Skip to content

Commit 4e603e1

Browse files
authored
fix: restrict reverse cast constraint propagation to safe conversions (#25531)
## Which issue does this PR close? Closes #25523. Related to #25407. ## Rationale for this change A filter on a cast can incorrectly make its input appear constant and remove a required sort. For example, `CAST(x AS INT) = 0` matches both -0.5 and 0.5, but casting the singleton result interval back to DOUBLE yields [0.0, 0.0]. The optimizer can then return the wrong order for `ORDER BY x DESC`. ## What changes are included in this PR? Gate reverse Cast constraint propagation through a common allowlist for all types. Preserve propagation for conversions recognized by `check_bigger_cast`, integer-to-integer casts, and Float32-to-Float64 casts. For other conversions, keep the existing input range instead of treating a cast back as an inverse. The conservative allowlist may reduce range refinement and pruning for safe conversions that are not yet recognized. Runtime CAST behavior and forward interval evaluation are unchanged. ## What is the testing strategy for this PR? - Extend existing `cast.slt` with float-to-integer, integer-to-Float32, timestamp-to-date, and string-to-integer cases. The first three reproduce incorrect sort elimination before the corresponding guards; the string case checks compatibility. - Add a table-driven test for permitted and rejected reverse propagation paths. - Check Float32 widening against actual Cast results at NaN, infinity, signed zero, subnormal, and rounding boundaries. - Passed 24 Cast unit tests (one existing test ignored), 46 filter tests, and four relevant SLT files. ## Are there any user-facing changes? Queries retain required sorting when cast constraints cannot safely determine the input range. No public API changes.
1 parent 04d683d commit 4e603e1

2 files changed

Lines changed: 177 additions & 4 deletions

File tree

  • datafusion

datafusion/physical-expr/src/expressions/cast.rs

Lines changed: 129 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -386,11 +386,13 @@ impl PhysicalExpr for CastExpr {
386386
interval: &Interval,
387387
children: &[&Interval],
388388
) -> Result<Option<Vec<Interval>>> {
389-
let child_interval = children[0];
390-
// Get child's datatype:
391-
let cast_type = child_interval.data_type();
389+
let source_type = children[0].data_type();
390+
let target_type = self.cast_type();
391+
if !can_propagate_cast_constraints(&source_type, target_type) {
392+
return Ok(Some(vec![]));
393+
}
392394
Ok(Some(vec![
393-
interval.cast_to(&cast_type, &DEFAULT_SAFE_CAST_OPTIONS)?,
395+
interval.cast_to(&source_type, &DEFAULT_SAFE_CAST_OPTIONS)?,
394396
]))
395397
}
396398

@@ -427,6 +429,19 @@ impl PhysicalExpr for CastExpr {
427429
}
428430
}
429431

432+
/// Whether output bounds can be cast back to `source` without excluding valid inputs.
433+
///
434+
/// Used for reverse constraint propagation through a cast from `source` to `target`.
435+
/// Many-to-one casts, such as Float64 to Int32, cannot generally be inverted this way:
436+
/// an output of 0 does not imply an input of 0.0.
437+
/// Returns false for unrecognized conversions so the input range remains unchanged.
438+
fn can_propagate_cast_constraints(source: &DataType, target: &DataType) -> bool {
439+
CastExpr::check_bigger_cast(target, source)
440+
|| (source.is_integer() && target.is_integer())
441+
// NaN bounds are unbounded; finite Float32 values widen exactly.
442+
|| (*source == Float32 && *target == Float64)
443+
}
444+
430445
#[cfg(feature = "proto")]
431446
impl CastExpr {
432447
/// Reconstruct a [`CastExpr`] from its protobuf representation.
@@ -595,10 +610,120 @@ mod tests {
595610
as_boolean_array, as_int64_array, as_string_array, as_struct_array,
596611
as_uint8_array,
597612
};
613+
use datafusion_common::rounding::{next_down, next_up};
598614
use datafusion_physical_expr_common::physical_expr::fmt_sql;
599615
use insta::assert_snapshot;
600616
use std::collections::HashMap;
601617

618+
#[test]
619+
fn test_cast_constraint_propagation() -> Result<()> {
620+
for (source, target, propagates) in [
621+
(Utf8, Int32, false),
622+
(Utf8View, Int32, false),
623+
(Timestamp(TimeUnit::Nanosecond, None), Date32, false),
624+
(Int32, Date32, true),
625+
(Date32, Int32, true),
626+
(Utf8, LargeUtf8, true),
627+
(Utf8, Utf8, true),
628+
(Float64, Int32, false),
629+
(Int64, Float32, false),
630+
(Float64, Float32, false),
631+
(Decimal128(4, 1), Decimal128(4, 0), false),
632+
(Decimal128(4, 1), Int32, false),
633+
(Float64, Decimal128(4, 1), false),
634+
(Int8, Int64, true),
635+
(Int64, Int8, true),
636+
(Int32, UInt32, true),
637+
(UInt32, Int32, true),
638+
(Int32, Float64, true),
639+
(Float32, Float64, true),
640+
(Decimal128(4, 1), Decimal128(4, 1), true),
641+
] {
642+
let schema = Schema::new(vec![Field::new("x", source.clone(), true)]);
643+
let expr = CastExpr::new(col("x", &schema)?, target.clone(), None);
644+
let input = Interval::make_unbounded(&source)?;
645+
let value = ScalarValue::Int32(Some(0)).cast_to(&target)?;
646+
let output = Interval::from(&value);
647+
let expected = if propagates {
648+
vec![Interval::from(&value.cast_to(&source)?)]
649+
} else {
650+
vec![]
651+
};
652+
assert_eq!(
653+
expr.propagate_constraints(&output, &[&input])?,
654+
Some(expected),
655+
"{source} -> {target}"
656+
);
657+
}
658+
Ok(())
659+
}
660+
661+
#[test]
662+
fn test_float_widening_constraint_boundaries() -> Result<()> {
663+
let mut values = vec![
664+
f32::NEG_INFINITY,
665+
-f32::MAX,
666+
-1.0,
667+
-f32::MIN_POSITIVE,
668+
-f32::from_bits(1),
669+
-0.0,
670+
0.0,
671+
f32::from_bits(1),
672+
f32::MIN_POSITIVE,
673+
1.0,
674+
f32::MAX,
675+
f32::INFINITY,
676+
];
677+
// Include both signs of signaling and quiet NaNs with distinct payloads.
678+
values.extend(
679+
[0x7f800001, 0x7f800002, 0x7fc00001, 0xff800001, 0xffc00001]
680+
.map(f32::from_bits),
681+
);
682+
values.extend([next_down(1.0f32), next_up(1.0f32)]);
683+
let schema = Arc::new(Schema::new(vec![Field::new("x", Float32, false)]));
684+
let batch = RecordBatch::try_new(
685+
Arc::clone(&schema),
686+
vec![Arc::new(Float32Array::from(values.clone()))],
687+
)?;
688+
let expr = CastExpr::new(col("x", &schema)?, Float64, None);
689+
let array = expr.evaluate(&batch)?.into_array(values.len())?;
690+
let widened = array.as_any().downcast_ref::<Float64Array>().unwrap();
691+
let mut bounds = vec![-f64::MAX, f64::MAX];
692+
for value in widened.values() {
693+
bounds.extend([next_down(*value), *value, next_up(*value)]);
694+
}
695+
// Midpoints exercise rounding back to Float32 in both directions.
696+
bounds.extend([
697+
f64::from(f32::from_bits(1)) / 2.0,
698+
-f64::from(f32::from_bits(1)) / 2.0,
699+
f64::midpoint(1.0, f64::from(next_up(1.0f32))),
700+
]);
701+
bounds.sort_by(f64::total_cmp);
702+
bounds.dedup_by(|a, b| a.to_bits() == b.to_bits());
703+
let input = Interval::make_unbounded(&Float32)?;
704+
for (i, lower) in bounds.iter().enumerate() {
705+
for upper in &bounds[i..] {
706+
let output = Interval::make(Some(*lower), Some(*upper))?;
707+
let propagated = expr.propagate_constraints(&output, &[&input])?.unwrap();
708+
assert_eq!(propagated.len(), 1);
709+
for (index, value) in values.iter().enumerate() {
710+
if output.contains_value(ScalarValue::Float64(Some(
711+
widened.value(index),
712+
)))? {
713+
assert!(
714+
propagated[0]
715+
.contains_value(ScalarValue::Float32(Some(*value)))?,
716+
"input bits={:08x}, output={output}, propagated={:?}",
717+
value.to_bits(),
718+
propagated[0]
719+
);
720+
}
721+
}
722+
}
723+
}
724+
Ok(())
725+
}
726+
602727
fn make_struct_array(fields: Fields, arrays: Vec<ArrayRef>) -> StructArray {
603728
StructArray::new(fields, arrays, None)
604729
}

datafusion/sqllogictest/test_files/cast.slt

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,54 @@ statement ok
9191
drop table t0;
9292

9393

94+
# Regression for https://github.com/apache/datafusion/issues/25523.
95+
statement ok
96+
SET datafusion.execution.target_partitions = 1;
97+
98+
statement ok
99+
CREATE TABLE lossy_cast (id INT, x DOUBLE, n BIGINT)
100+
AS VALUES (1, -0.5, 16777216), (2, 0.5, 16777217), (3, NULL, NULL);
101+
102+
# A singleton cast result does not imply a singleton input.
103+
query I
104+
SELECT id FROM lossy_cast WHERE CAST(x AS INT) = 0 ORDER BY x DESC;
105+
----
106+
2
107+
1
108+
109+
query I
110+
SELECT id FROM lossy_cast WHERE CAST(n AS REAL) = 16777216::REAL ORDER BY n DESC;
111+
----
112+
2
113+
1
114+
115+
statement ok
116+
DROP TABLE lossy_cast;
117+
118+
statement ok
119+
CREATE TABLE lossy_non_numeric (id INT, t TIMESTAMP, s VARCHAR)
120+
AS VALUES (1, TIMESTAMP '2026-01-01 01:00:00', '01'),
121+
(2, TIMESTAMP '2026-01-01 02:00:00', '1');
122+
123+
query I
124+
SELECT id FROM lossy_non_numeric
125+
WHERE CAST(t AS DATE) = DATE '2026-01-01' ORDER BY t DESC;
126+
----
127+
2
128+
1
129+
130+
query I
131+
SELECT id FROM lossy_non_numeric WHERE CAST(s AS INT) = 1 ORDER BY s DESC;
132+
----
133+
2
134+
1
135+
136+
statement ok
137+
DROP TABLE lossy_non_numeric;
138+
139+
statement ok
140+
SET datafusion.execution.target_partitions = 4;
141+
94142
# ensure that automatically casting with "datafusion.optimizer.expand_views_at_output" does not
95143
# change the column name
96144

0 commit comments

Comments
 (0)