From c2f6158673c44c37d30a68be5bcce0584a6e5934 Mon Sep 17 00:00:00 2001 From: Nik <84yk8btb9f@privaterelay.appleid.com> Date: Sun, 25 Oct 2026 08:09:05 +0200 Subject: [PATCH] fix: compare Duration and Interval as exact elapsed time in comparisons Comparing a Duration (e.g. ts1 - ts2) with an INTERVAL that has a days or months component was decided by the wrong field: the mixed comparison coerced both sides to Interval(MonthDayNano), whose derived ordering compares months, then days, then nanoseconds, so a nanoseconds-only duration always compared incorrectly against any interval with non-zero days (e.g. a 10-day duration satisfied <= INTERVAL '3 days'). Route mixed Duration/Interval comparisons through a dedicated coercion that compares exact elapsed time by coercing both sides to Duration(Nanosecond). Intervals whose months and days are zero compare exactly; any other interval fails loudly when cast (during constant folding or at execution) instead of silently comparing on the wrong field. Interval(YearMonth) and Interval(DayTime) have no exact conversion and fail at planning, as before. Duration-vs-Duration and Interval-vs-Interval comparison coercion is unchanged. Closes #26671 --- .../expr-common/src/type_coercion/binary.rs | 50 +++++++++++ .../type_coercion/binary/tests/comparison.rs | 79 +++++++++++++++++ .../test_files/datetime/interval.slt | 88 +++++++++++++++++++ 3 files changed, 217 insertions(+) diff --git a/datafusion/expr-common/src/type_coercion/binary.rs b/datafusion/expr-common/src/type_coercion/binary.rs index 5355cebd7544e..e3dcf6bb694b4 100644 --- a/datafusion/expr-common/src/type_coercion/binary.rs +++ b/datafusion/expr-common/src/type_coercion/binary.rs @@ -1035,6 +1035,17 @@ where // same type => equality is possible return Some(lhs_type.clone()); } + // Mixed Duration/Interval comparisons need exact elapsed-time semantics, + // so they are handled here rather than falling through to + // `temporal_coercion`, which unifies any Interval with any Duration as + // `Interval(MonthDayNano)`. + if matches!( + (lhs_type, rhs_type), + (DataType::Duration(_), DataType::Interval(_)) + | (DataType::Interval(_), DataType::Duration(_)) + ) { + return duration_interval_comparison_coercion(lhs_type, rhs_type); + } binary_numeric_coercion(lhs_type, rhs_type) .or_else(|| dictionary_coercion(lhs_type, rhs_type, true, coerce_fn)) .or_else(|| ree_coercion(lhs_type, rhs_type, true, coerce_fn)) @@ -2225,6 +2236,45 @@ fn temporal_math_coercion( } } +/// Coercion for comparisons between a `Duration` and an `Interval`. +/// +/// Durations measure exact elapsed time, while intervals can carry calendar +/// units (months, days) whose absolute length is not fixed. Comparing the two +/// as `Interval(MonthDayNano)` would use its lexicographic +/// months-then-days-then-nanoseconds ordering, so any interval with a +/// non-zero months or days field would be decided against a duration's +/// nanoseconds-only representation on the wrong field (e.g. a 10-day +/// duration would compare as less than `INTERVAL '3 days'`). +/// +/// Instead, compare exact elapsed time by coercing both sides to +/// `Duration(Nanosecond)`. Arrow only supports casting +/// `Interval(MonthDayNano)` to a duration, and only for values whose months +/// and days are zero, failing loudly otherwise ("Cannot convert interval +/// containing non-zero months or days to duration"), which keeps +/// pure-nanosecond intervals comparable while months- or days-bearing +/// intervals error rather than silently comparing on the wrong field. +/// `Interval(YearMonth)` and `Interval(DayTime)` have no duration cast at +/// all, so comparisons against them are unsupported. +fn duration_interval_comparison_coercion( + lhs_type: &DataType, + rhs_type: &DataType, +) -> Option { + use arrow::datatypes::DataType::*; + use arrow::datatypes::IntervalUnit::*; + use arrow::datatypes::TimeUnit::*; + + match (lhs_type, rhs_type) { + (Duration(_), Interval(MonthDayNano)) | (Interval(MonthDayNano), Duration(_)) => { + Some(Duration(Nanosecond)) + } + (Duration(_), Interval(YearMonth | DayTime)) + | (Interval(YearMonth | DayTime), Duration(_)) => None, + // Not a mixed Duration/Interval pair; the caller only routes mixed + // pairs here, so reaching this arm would be a caller bug. + _ => None, + } +} + fn temporal_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option { use arrow::datatypes::DataType::*; use arrow::datatypes::IntervalUnit::*; diff --git a/datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs b/datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs index 5d307dc1b90fd..7abb082854186 100644 --- a/datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs +++ b/datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs @@ -1245,3 +1245,82 @@ fn test_comparison_coercion_with_session_timezone() { Some(DataType::Utf8) ); } + +/// Duration vs Interval comparisons coerce to exact `Duration(Nanosecond)` +/// comparison rather than `Interval(MonthDayNano)`, whose lexicographic +/// (months, days, nanoseconds) ordering decides on the wrong field whenever +/// the interval has a non-zero months or days component (#26671). +#[test] +fn test_duration_interval_comparison_coercion() -> Result<()> { + // All comparison operators, both operand orders, and every duration unit: + // `Interval(MonthDayNano)` is the only interval unit with a duration + // cast, so both sides coerce to exact nanosecond durations. Values with + // non-zero months or days fail that cast loudly instead of comparing on + // the wrong field. + for op in [ + Operator::Eq, + Operator::NotEq, + Operator::Lt, + Operator::LtEq, + Operator::Gt, + Operator::GtEq, + Operator::IsDistinctFrom, + Operator::IsNotDistinctFrom, + ] { + for duration in [ + DataType::Duration(Second), + DataType::Duration(Millisecond), + DataType::Duration(Microsecond), + DataType::Duration(Nanosecond), + ] { + test_coercion_binary_rule!( + duration, + DataType::Interval(MonthDayNano), + op, + DataType::Duration(Nanosecond) + ); + test_coercion_binary_rule!( + DataType::Interval(MonthDayNano), + duration, + op, + DataType::Duration(Nanosecond) + ); + } + + // `Interval(YearMonth)` (months only) and `Interval(DayTime)` (days) + // have no exact duration conversion, so the comparison is rejected + // during planning. + for interval in [ + DataType::Interval(arrow::datatypes::IntervalUnit::YearMonth), + DataType::Interval(arrow::datatypes::IntervalUnit::DayTime), + ] { + assert!( + BinaryTypeCoercer::new(&DataType::Duration(Nanosecond), &op, &interval) + .get_input_types() + .is_err() + ); + assert!( + BinaryTypeCoercer::new(&interval, &op, &DataType::Duration(Nanosecond)) + .get_input_types() + .is_err() + ); + } + } + + // Duration vs Duration and Interval vs Interval comparisons keep coercing + // through `Interval(MonthDayNano)` as before. + test_coercion_binary_rule!( + DataType::Duration(Second), + DataType::Duration(Nanosecond), + Operator::Lt, + DataType::Interval(MonthDayNano) + ); + test_coercion_binary_rule!( + DataType::Interval(arrow::datatypes::IntervalUnit::YearMonth), + DataType::Interval(MonthDayNano), + Operator::Lt, + DataType::Interval(MonthDayNano) + ); + + Ok(()) +} diff --git a/datafusion/sqllogictest/test_files/datetime/interval.slt b/datafusion/sqllogictest/test_files/datetime/interval.slt index ce5d52c58517d..1188258adcb10 100644 --- a/datafusion/sqllogictest/test_files/datetime/interval.slt +++ b/datafusion/sqllogictest/test_files/datetime/interval.slt @@ -591,3 +591,91 @@ true true true true true true statement ok drop table t + +# https://github.com/apache/datafusion/issues/26671 +# Duration vs Interval comparisons compare exact elapsed time by coercing both +# sides to Duration(Nanosecond). Only intervals whose months and days are all +# zero (e.g. hour/minute/second intervals) convert exactly; any other interval +# errors instead of silently deciding the comparison on the wrong field. + +### duration (scalar) cmp interval (scalar), pure-nanosecond interval +query BBBBBB +select + (timestamp '2026-01-11' - timestamp '2026-01-01') = interval '240 hours', + (timestamp '2026-01-11' - timestamp '2026-01-01') != interval '239 hours', + (timestamp '2026-01-11' - timestamp '2026-01-01') < interval '241 hours', + (timestamp '2026-01-11' - timestamp '2026-01-01') <= interval '240 hours', + (timestamp '2026-01-11' - timestamp '2026-01-01') > interval '239 hours', + (timestamp '2026-01-11' - timestamp '2026-01-01') >= interval '1 hour'; +---- +true true true true true true + +### interval (scalar) cmp duration (scalar) +query B +select interval '240 hours' = (timestamp '2026-01-11' - timestamp '2026-01-01'); +---- +true + +### intervals with a days or months component are not exactly comparable to durations +query error Cannot convert interval containing non-zero months or days to duration +select (timestamp '2026-01-11' - timestamp '2026-01-01') <= interval '3 days'; + +query error Cannot convert interval containing non-zero months or days to duration +select (timestamp '2026-01-11' - timestamp '2026-01-01') <= interval '1 month'; + +query error Cannot convert interval containing non-zero months or days to duration +select interval '3 days' >= (timestamp '2026-01-11' - timestamp '2026-01-01'); + +### runtime path: duration (timestamp - timestamp) cmp interval literal +statement ok +create table duration_interval_t(t1 timestamp, t2 timestamp) as values + (timestamp '2026-01-11', timestamp '2026-01-01'), + (timestamp '2026-01-01 00:30:00', timestamp '2026-01-01'); + +query BB +select (t1 - t2) >= interval '1 hour', (t1 - t2) < interval '240 hours' from duration_interval_t order by t1; +---- +false true +true false + +### runtime path: interval column with days errors at execution +statement ok +create table interval_col_t(i interval) as values (interval '2 hours'), (interval '3 days'); + +query error Cannot convert interval containing non-zero months or days to duration +select (timestamp '2026-01-11' - timestamp '2026-01-01') <= i from interval_col_t; + +### runtime path: pure-nanosecond interval column compares exactly +statement ok +create table interval_col_pure_t(i interval) as values (interval '2 hours'), (interval '250 hours'); + +query BB +select + (timestamp '2026-01-11' - timestamp '2026-01-01') >= i, + (timestamp '2026-01-11' - timestamp '2026-01-01') < i +from interval_col_pure_t order by i; +---- +true false +false true + +### IN list with a mixed duration/interval pair +query B +select (timestamp '2026-01-11' - timestamp '2026-01-01') in (interval '239 hours', interval '240 hours'); +---- +true + +### Interval(YearMonth) and Interval(DayTime) have no exact duration conversion +query error Cannot infer common argument type for comparison operation +select (timestamp '2026-01-11' - timestamp '2026-01-01') <= arrow_cast('13 months', 'Interval(YearMonth)'); + +query error Cannot infer common argument type for comparison operation +select arrow_cast('3 days', 'Interval(DayTime)') >= (timestamp '2026-01-11' - timestamp '2026-01-01'); + +statement ok +drop table duration_interval_t; + +statement ok +drop table interval_col_t; + +statement ok +drop table interval_col_pure_t;