This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25520-99af44a5e620f899bb6796b9fbdcb6b00d33c6e1 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 923d64fd5448121f538612a7173c99108c108ae7 Author: Liang-Chi Hsieh <[email protected]> AuthorDate: Sun Sep 20 16:30:46 2026 +0000 fix: reject mixed-sign intervals in range functions (#25520) ## Which issue does this PR close? N/A ## Rationale for this change Timestamp and date interval steps contain independent month, day, and nanosecond components. When those components have mixed signs, the effective direction can depend on the current calendar date. For example, adding `INTERVAL '1 MONTH -29 DAY'` to `2024-01-31` produces the same date. Such steps could cause `generate_series` and `range` to return an empty result unexpectedly or fail to terminate. ## What changes are included in this PR? Following DuckDB's behavior, reject interval steps containing both positive and negative components. The validation applies consistently to: - Scalar/list and table-function forms - `generate_series` and `range` - DATE and TIMESTAMP arguments Zero-step and same-sign interval behavior remains unchanged. ## What is the testing strategy for this PR? - Added unit tests for positive, negative, and mixed-sign interval components. - Added SQL logic tests covering both mixed-sign directions. - Tested scalar/list and table-function forms. - Ran `cargo fmt --all`. - Ran Clippy with `-D warnings` for both affected crates. - Ran the relevant `table_functions.slt` and `array_range.slt` tests. ## Are there any user-facing changes? Yes. Mixed-sign interval steps are now rejected with an error instead of potentially returning an unexpected empty result or producing a non-terminating series. There are no public API changes. --- datafusion/functions-nested/src/range.rs | 31 +++++++++++++--------- datafusion/functions-table/src/generate_series.rs | 23 +++++++++++++--- .../sqllogictest/test_files/array/array_range.slt | 9 +++++++ .../sqllogictest/test_files/table_functions.slt | 9 +++++++ 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/datafusion/functions-nested/src/range.rs b/datafusion/functions-nested/src/range.rs index f44b0480aa..264bd70cff 100644 --- a/datafusion/functions-nested/src/range.rs +++ b/datafusion/functions-nested/src/range.rs @@ -50,7 +50,6 @@ use datafusion_expr::{ TypeSignature, TypeSignatureClass, Volatility, }; use datafusion_macros::user_doc; -use std::cmp::Ordering; use std::iter::from_fn; use std::str::FromStr; use std::sync::Arc; @@ -64,6 +63,22 @@ make_udf_expr_and_func!( Range::new ); +fn interval_step_is_negative( + name: &str, + months: i32, + days: i32, + nanoseconds: i64, +) -> Result<bool> { + let has_positive_component = months > 0 || days > 0 || nanoseconds > 0; + let has_negative_component = months < 0 || days < 0 || nanoseconds < 0; + + if has_positive_component && has_negative_component { + return exec_err!("Interval argument to {name} must not have mixed signs"); + } + + Ok(has_negative_component) +} + make_udf_expr_and_func!( GenSeries, gen_series, @@ -374,10 +389,11 @@ impl Range { let stop = stop.value(idx); let step = step.value(idx); - let (months, days, _) = IntervalMonthDayNanoType::to_parts(step); + let (months, days, nanoseconds) = IntervalMonthDayNanoType::to_parts(step); if months == 0 && days == 0 { return exec_err!("Cannot generate date range less than 1 day."); } + let neg = interval_step_is_negative(self.name(), months, days, nanoseconds)?; let stop = if !self.include_upper_bound { Date32Type::subtract_month_day_nano_opt(stop, step).ok_or_else(|| { @@ -390,7 +406,6 @@ impl Range { stop }; - let neg = months < 0 || days < 0; let mut new_date = Some(start); let values = from_fn(|| { @@ -459,15 +474,7 @@ impl Range { if months == 0 && days == 0 && ns == 0 { return exec_err!("Interval argument to {} must not be 0", self.name()); } - - let neg = TimestampNanosecondType::add_month_day_nano(start, step, start_tz) - .ok_or_else(|| { - exec_datafusion_err!( - "Cannot generate timestamp range where start + step overflows" - ) - })? - .cmp(&start) - == Ordering::Less; + let neg = interval_step_is_negative(self.name(), months, days, ns)?; let stop_dt = as_datetime_with_timezone::<TimestampNanosecondType>(stop, stop_tz) diff --git a/datafusion/functions-table/src/generate_series.rs b/datafusion/functions-table/src/generate_series.rs index 94e69e17c3..bca6081ebd 100644 --- a/datafusion/functions-table/src/generate_series.rs +++ b/datafusion/functions-table/src/generate_series.rs @@ -583,9 +583,15 @@ fn timestamp_arg_to_nanos( } fn validate_interval_step(step: IntervalMonthDayNano) -> Result<()> { - if step.months == 0 && step.days == 0 && step.nanoseconds == 0 { + let has_positive_component = step.months > 0 || step.days > 0 || step.nanoseconds > 0; + let has_negative_component = step.months < 0 || step.days < 0 || step.nanoseconds < 0; + + if !has_positive_component && !has_negative_component { return plan_err!("Step interval cannot be zero"); } + if has_positive_component && has_negative_component { + return plan_err!("Step interval cannot have mixed signs"); + } Ok(()) } @@ -939,14 +945,25 @@ impl TableFunctionImpl for RangeFunc { mod generate_series_tests { use std::sync::Arc; - use arrow::datatypes::{DataType, Field, Schema}; + use arrow::datatypes::{DataType, Field, IntervalMonthDayNano, Schema}; use datafusion_common::Result; use datafusion_physical_plan::memory::LazyBatchGenerator; use crate::generate_series::{ - GenSeriesArgs, GenerateSeriesTable, GenericSeriesState, + GenSeriesArgs, GenerateSeriesTable, GenericSeriesState, validate_interval_step, }; + #[test] + fn rejects_mixed_sign_interval_steps() { + assert!(validate_interval_step(IntervalMonthDayNano::new(1, -29, 0)).is_err()); + assert!(validate_interval_step(IntervalMonthDayNano::new(-1, 29, 0)).is_err()); + + validate_interval_step(IntervalMonthDayNano::new(1, 29, 1)) + .expect("positive interval should be valid"); + validate_interval_step(IntervalMonthDayNano::new(-1, -29, -1)) + .expect("negative interval should be valid"); + } + #[test] fn generate_series_rejects_zero_batch_size() { let schema = Arc::new(Schema::new(vec![Field::new("a", DataType::Int64, false)])); diff --git a/datafusion/sqllogictest/test_files/array/array_range.slt b/datafusion/sqllogictest/test_files/array/array_range.slt index b2a39634ef..022740239a 100644 --- a/datafusion/sqllogictest/test_files/array/array_range.slt +++ b/datafusion/sqllogictest/test_files/array/array_range.slt @@ -359,6 +359,15 @@ select generate_series(1, 1, 0); query error DataFusion error: Execution error: Interval argument to generate_series must not be 0 select generate_series(TIMESTAMP '2000-01-02', TIMESTAMP '2000-01-01', INTERVAL '0' MINUTE); +# Mixed-sign interval components can change direction depending on the current +# calendar date, so they are rejected rather than risking a non-terminating +# series. +query error DataFusion error: Execution error: Interval argument to generate_series must not have mixed signs +select generate_series(TIMESTAMP '2024-01-31', TIMESTAMP '2024-02-01', INTERVAL '1 MONTH -29 DAY'); + +query error DataFusion error: Execution error: Interval argument to range must not have mixed signs +select range(DATE '2024-02-01', DATE '2024-01-01', INTERVAL '-1 MONTH 29 DAY'); + # Range too large to materialize should error instead of panicking query error DataFusion error: Execution error: Range too large to materialize select generate_series(0, 9223372036854775807); diff --git a/datafusion/sqllogictest/test_files/table_functions.slt b/datafusion/sqllogictest/test_files/table_functions.slt index 3b9164df3d..168f5a0796 100644 --- a/datafusion/sqllogictest/test_files/table_functions.slt +++ b/datafusion/sqllogictest/test_files/table_functions.slt @@ -259,6 +259,15 @@ SELECT * FROM generate_series(DATE '2262-04-10', DATE '2262-04-11', INTERVAL '2' ---- 2262-04-10T00:00:00 +# Mixed-sign interval components do not have a stable direction because month +# arithmetic depends on the current calendar date. Reject them rather than +# returning an empty series or failing to make progress. +statement error DataFusion error: Error during planning: Step interval cannot have mixed signs +SELECT * FROM generate_series(TIMESTAMP '2024-01-31', TIMESTAMP '2024-02-01', INTERVAL '1 MONTH -29 DAY') + +statement error DataFusion error: Error during planning: Step interval cannot have mixed signs +SELECT * FROM range(DATE '2024-02-01', DATE '2024-01-01', INTERVAL '-1 MONTH 29 DAY') + # # Timestamp bounds of every precision # https://github.com/apache/datafusion/issues/25169 --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
