From 0da90898af50468c0ac475972f41893bcf5bd116 Mon Sep 17 00:00:00 2001 From: Namgung Chan <9511chn@gmail.com> Date: Thu, 23 Jul 2026 02:15:22 +0900 Subject: [PATCH 1/3] fix: exclude precision-losing integer-to-float conversions from CastExpr::check_bigger_cast (#23808) --- .../physical-expr/src/expressions/cast.rs | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/datafusion/physical-expr/src/expressions/cast.rs b/datafusion/physical-expr/src/expressions/cast.rs index 26f06b546ad1d..871e001ec2f69 100644 --- a/datafusion/physical-expr/src/expressions/cast.rs +++ b/datafusion/physical-expr/src/expressions/cast.rs @@ -179,11 +179,8 @@ impl CastExpr { | (UInt8, UInt16 | UInt32 | UInt64) | (UInt16, UInt32 | UInt64) | (UInt32, UInt64) - | ( - Int8 | Int16 | Int32 | UInt8 | UInt16 | UInt32, - Float32 | Float64 - ) - | (Int64 | UInt64, Float64) + | (Int8 | Int16 | UInt8 | UInt16, Float32) + | (Int8 | Int16 | Int32 | UInt8 | UInt16 | UInt32, Float64) | (Utf8, LargeUtf8) ) } @@ -1369,4 +1366,24 @@ mod proto_tests { DataFusionError::Internal(msg) if msg.contains("call 1") )); } + + #[test] + fn test_check_bigger_cast_precision_loss() { + use DataType::*; + + // Exact conversions without precision loss + assert!(CastExpr::check_bigger_cast(&Int16, &Int8)); + assert!(CastExpr::check_bigger_cast(&Int64, &Int32)); + assert!(CastExpr::check_bigger_cast(&Float32, &Int16)); + assert!(CastExpr::check_bigger_cast(&Float32, &UInt16)); + assert!(CastExpr::check_bigger_cast(&Float64, &Int32)); + assert!(CastExpr::check_bigger_cast(&Float64, &UInt32)); + assert!(CastExpr::check_bigger_cast(&LargeUtf8, &Utf8)); + + // Precision-losing int-to-float conversions should return false + assert!(!CastExpr::check_bigger_cast(&Float32, &Int32)); + assert!(!CastExpr::check_bigger_cast(&Float32, &UInt32)); + assert!(!CastExpr::check_bigger_cast(&Float64, &Int64)); + assert!(!CastExpr::check_bigger_cast(&Float64, &UInt64)); + } } From 860f79cb9ee894c5481a5a1e4767d088f0a35ce0 Mon Sep 17 00:00:00 2001 From: Namgung Chan <9511chn@gmail.com> Date: Sat, 25 Jul 2026 16:10:49 +0900 Subject: [PATCH 2/3] test: add sqllogictest using CSV data for int-to-float cast precision bug --- .../data/int_to_float_cast_precision.csv | 3 + .../test_files/monotonic_projection_test.slt | 63 +++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 datafusion/core/tests/data/int_to_float_cast_precision.csv diff --git a/datafusion/core/tests/data/int_to_float_cast_precision.csv b/datafusion/core/tests/data/int_to_float_cast_precision.csv new file mode 100644 index 0000000000000..187d7affca616 --- /dev/null +++ b/datafusion/core/tests/data/int_to_float_cast_precision.csv @@ -0,0 +1,3 @@ +k,v +1,16777217 +2,16777216 diff --git a/datafusion/sqllogictest/test_files/monotonic_projection_test.slt b/datafusion/sqllogictest/test_files/monotonic_projection_test.slt index 0045e51715980..71e5fbc08e3eb 100644 --- a/datafusion/sqllogictest/test_files/monotonic_projection_test.slt +++ b/datafusion/sqllogictest/test_files/monotonic_projection_test.slt @@ -252,3 +252,66 @@ ORDER BY a, b; ---- a 1 a0 1 + +# Test that precision-losing int-to-float casts do not invalidate suffix sort keys. +# +# When CAST(Int32 AS Float32) collapses distinct integer values (e.g., 16777216 and +# 16777217 both become 16777216.0), the suffix sort key (k) must still be sorted +# correctly. Before the fix, the optimizer incorrectly reused the pre-existing sort +# order and dropped the SortExec, producing wrong results. +# +# t1 is declared with a sort order, t2 is not — their results should be identical +# since CAST(v AS FLOAT) is not injective for 32-bit integers. +statement ok +CREATE EXTERNAL TABLE t1_int_float (k int, v int) +STORED AS CSV +WITH ORDER (v DESC, k DESC) +LOCATION '../core/tests/data/int_to_float_cast_precision.csv' +OPTIONS ('format.has_header' 'true'); + +statement ok +CREATE EXTERNAL TABLE t2_int_float (k int, v int) +STORED AS CSV +LOCATION '../core/tests/data/int_to_float_cast_precision.csv' +OPTIONS ('format.has_header' 'true'); + +# Both queries must return the same result: k=2 before k=1. +# (v_=16777216.0 for both rows; when tied on v_, DESC on k means k=2 comes first) +query IR +SELECT k, cast(v as float) v_ FROM t1_int_float ORDER BY v_ DESC, k DESC; +---- +2 16777216 +1 16777216 + +query IR +SELECT k, cast(v as float) v_ FROM t2_int_float ORDER BY v_ DESC, k DESC; +---- +2 16777216 +1 16777216 + +# Widening cast (Int32 -> Int64) is strictly 1-to-1, so the optimizer CAN +# legally reuse the pre-existing sort order and omit a SortExec. +statement ok +CREATE EXTERNAL TABLE t3_int_bigint (k int, v int) +STORED AS CSV +WITH ORDER (v DESC, k DESC) +LOCATION '../core/tests/data/int_to_float_cast_precision.csv' +OPTIONS ('format.has_header' 'true'); + +# CAST(Int32 AS BIGINT) is injective, so suffix key ordering is preserved. +query II +SELECT k, cast(v as bigint) v_ FROM t3_int_bigint ORDER BY v_ DESC, k DESC; +---- +1 16777217 +2 16777216 + +# Cleanup +statement ok +DROP TABLE t1_int_float; + +statement ok +DROP TABLE t2_int_float; + +statement ok +DROP TABLE t3_int_bigint; + From 0bc07d5cada432913ef3d8d8c70749c7de560709 Mon Sep 17 00:00:00 2001 From: Namgung Chan <9511chn@gmail.com> Date: Sat, 25 Jul 2026 16:18:27 +0900 Subject: [PATCH 3/3] test: add assertions for signed-unsigned casts in check_bigger_cast --- .../physical-expr/src/expressions/cast.rs | 45 ++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/datafusion/physical-expr/src/expressions/cast.rs b/datafusion/physical-expr/src/expressions/cast.rs index 871e001ec2f69..8be2e187d72f7 100644 --- a/datafusion/physical-expr/src/expressions/cast.rs +++ b/datafusion/physical-expr/src/expressions/cast.rs @@ -1205,6 +1205,31 @@ mod tests { Ok(()) } + + #[test] + fn test_check_bigger_cast_precision_loss() { + use DataType::*; + + // Exact conversions without precision loss + assert!(CastExpr::check_bigger_cast(&Int16, &Int8)); + assert!(CastExpr::check_bigger_cast(&Int64, &Int32)); + assert!(CastExpr::check_bigger_cast(&Float32, &Int16)); + assert!(CastExpr::check_bigger_cast(&Float32, &UInt16)); + assert!(CastExpr::check_bigger_cast(&Float64, &Int32)); + assert!(CastExpr::check_bigger_cast(&Float64, &UInt32)); + assert!(CastExpr::check_bigger_cast(&LargeUtf8, &Utf8)); + + // Precision-losing int-to-float conversions should return false + assert!(!CastExpr::check_bigger_cast(&Float32, &Int32)); + assert!(!CastExpr::check_bigger_cast(&Float32, &UInt32)); + assert!(!CastExpr::check_bigger_cast(&Float64, &Int64)); + assert!(!CastExpr::check_bigger_cast(&Float64, &UInt64)); + + // Signed <-> Unsigned conversions should return false (not order-preserving due to negative values) + assert!(!CastExpr::check_bigger_cast(&UInt16, &Int8)); + assert!(!CastExpr::check_bigger_cast(&UInt32, &Int16)); + assert!(!CastExpr::check_bigger_cast(&Int16, &UInt8)); + } } /// Tests for the `try_to_proto` / `try_from_proto` hooks. @@ -1366,24 +1391,4 @@ mod proto_tests { DataFusionError::Internal(msg) if msg.contains("call 1") )); } - - #[test] - fn test_check_bigger_cast_precision_loss() { - use DataType::*; - - // Exact conversions without precision loss - assert!(CastExpr::check_bigger_cast(&Int16, &Int8)); - assert!(CastExpr::check_bigger_cast(&Int64, &Int32)); - assert!(CastExpr::check_bigger_cast(&Float32, &Int16)); - assert!(CastExpr::check_bigger_cast(&Float32, &UInt16)); - assert!(CastExpr::check_bigger_cast(&Float64, &Int32)); - assert!(CastExpr::check_bigger_cast(&Float64, &UInt32)); - assert!(CastExpr::check_bigger_cast(&LargeUtf8, &Utf8)); - - // Precision-losing int-to-float conversions should return false - assert!(!CastExpr::check_bigger_cast(&Float32, &Int32)); - assert!(!CastExpr::check_bigger_cast(&Float32, &UInt32)); - assert!(!CastExpr::check_bigger_cast(&Float64, &Int64)); - assert!(!CastExpr::check_bigger_cast(&Float64, &UInt64)); - } }