From 26cb0194dce91f284ad92acdbd631d6f362ce060 Mon Sep 17 00:00:00 2001 From: Dennis Felsing Date: Fri, 24 Jul 2026 08:45:40 +0200 Subject: [PATCH 1/6] sql: Fix BC year numbering in extract and make_timestamp (#37833) EXTRACT(YEAR ...) on dates and timestamps returned chrono's astronomical year numbering, where 1 BC is year 0, so BC years were off by one: 1 BC must be year -1 and there is no year 0. make_timestamp passed its year argument straight to chrono, accepting year 0 and shifting BC years by one. It also returned NULL for invalid field values instead of erroring, and rejected 24:00:00 and 60 seconds instead of rolling them over into the next day or minute. Closes: SQL-467 Co-authored-by: Claude Fable 5 --- src/expr/src/scalar/func/impls/date.rs | 2 +- src/expr/src/scalar/func/impls/timestamp.rs | 2 +- src/expr/src/scalar/func/variadic.rs | 62 ++++++++++----------- src/repr/src/adt/timestamp.rs | 8 +++ test/sqllogictest/dates-times.slt | 50 +++++++++++++++++ test/sqllogictest/not-null-propagation.slt | 23 +++++++- 6 files changed, 111 insertions(+), 36 deletions(-) diff --git a/src/expr/src/scalar/func/impls/date.rs b/src/expr/src/scalar/func/impls/date.rs index f36c887df72e1..8da16e5721e6c 100644 --- a/src/expr/src/scalar/func/impls/date.rs +++ b/src/expr/src/scalar/func/impls/date.rs @@ -139,7 +139,7 @@ pub fn extract_date_inner(units: DateTimeUnits, date: NaiveDate) -> Result Ok(Numeric::from(date.millennium())), DateTimeUnits::Century => Ok(Numeric::from(date.century())), DateTimeUnits::Decade => Ok(Numeric::from(date.decade())), - DateTimeUnits::Year => Ok(Numeric::from(date.year())), + DateTimeUnits::Year => Ok(Numeric::from(date.extract_year())), DateTimeUnits::Quarter => Ok(Numeric::from(date.quarter())), DateTimeUnits::Week => Ok(Numeric::from(date.iso_week_number())), DateTimeUnits::Month => Ok(Numeric::from(date.month())), diff --git a/src/expr/src/scalar/func/impls/timestamp.rs b/src/expr/src/scalar/func/impls/timestamp.rs index 5648020899542..38084ba147568 100644 --- a/src/expr/src/scalar/func/impls/timestamp.rs +++ b/src/expr/src/scalar/func/impls/timestamp.rs @@ -417,7 +417,7 @@ where DateTimeUnits::Millennium => Ok(D::from(ts.millennium())), DateTimeUnits::Century => Ok(D::from(ts.century())), DateTimeUnits::Decade => Ok(D::from(ts.decade())), - DateTimeUnits::Year => Ok(D::from(ts.year())), + DateTimeUnits::Year => Ok(D::from(ts.extract_year())), DateTimeUnits::Quarter => Ok(D::from(ts.quarter())), DateTimeUnits::Week => Ok(D::from(ts.iso_week_number())), DateTimeUnits::Month => Ok(D::from(ts.month())), diff --git a/src/expr/src/scalar/func/variadic.rs b/src/expr/src/scalar/func/variadic.rs index 125784661cc36..1f1164272ad9d 100644 --- a/src/expr/src/scalar/func/variadic.rs +++ b/src/expr/src/scalar/func/variadic.rs @@ -1056,7 +1056,6 @@ fn make_mz_acl_item( } #[sqlfunc(sqlname = "makets")] -// TODO(benesch): remove potentially dangerous usage of `as`. #[allow(clippy::as_conversions)] fn make_timestamp( year: i64, @@ -1066,36 +1065,37 @@ fn make_timestamp( minute: i64, second_float: f64, ) -> Result>, EvalError> { - let year: i32 = match year.try_into() { - Ok(year) => year, - Err(_) => return Ok(None), - }; - let month: u32 = match month.try_into() { - Ok(month) => month, - Err(_) => return Ok(None), - }; - let day: u32 = match day.try_into() { - Ok(day) => day, - Err(_) => return Ok(None), - }; - let hour: u32 = match hour.try_into() { - Ok(hour) => hour, - Err(_) => return Ok(None), - }; - let minute: u32 = match minute.try_into() { - Ok(minute) => minute, - Err(_) => return Ok(None), - }; - let second = second_float as u32; - let micros = ((second_float - second as f64) * 1_000_000.0) as u32; - let date = match NaiveDate::from_ymd_opt(year, month, day) { - Some(date) => date, - None => return Ok(None), - }; - let timestamp = match date.and_hms_micro_opt(hour, minute, second, micros) { - Some(timestamp) => timestamp, - None => return Ok(None), - }; + // NOTE: This function never returns `Ok(None)`. The `Option` stays so + // that the derived `introduces_nulls` (and with it the nullability of + // existing objects planned with this function) does not change. + + // Negative years are BC and there is no year 0. Chrono's astronomical + // numbering has 1 BC as year 0, so BC years shift by one. + if year == 0 { + return Err(EvalError::DateOutOfRange); + } + let year = if year < 0 { year + 1 } else { year }; + let year: i32 = year.try_into().map_err(|_| EvalError::DateOutOfRange)?; + let month: u32 = month.try_into().map_err(|_| EvalError::DateOutOfRange)?; + let day: u32 = day.try_into().map_err(|_| EvalError::DateOutOfRange)?; + let date = NaiveDate::from_ymd_opt(year, month, day).ok_or(EvalError::DateOutOfRange)?; + // Allow exactly 24:00:00 (rolls over to midnight of the next day) and + // 60 seconds (rolls over into the next minute). + if !(0..=24).contains(&hour) + || !(0..=59).contains(&minute) + || !(0.0..=60.0).contains(&second_float) + || (hour == 24 && (minute > 0 || second_float > 0.0)) + { + return Err(EvalError::TimestampOutOfRange); + } + // The cast is exact: second_float is in [0, 60]. + let micros = (second_float * 1_000_000.0).round() as i64; + let time = chrono::Duration::microseconds((hour * 60 + minute) * 60 * 1_000_000 + micros); + let timestamp = date + .and_hms_opt(0, 0, 0) + .unwrap() + .checked_add_signed(time) + .ok_or(EvalError::TimestampOutOfRange)?; Ok(Some(timestamp.try_into()?)) } diff --git a/src/repr/src/adt/timestamp.rs b/src/repr/src/adt/timestamp.rs index a3b0853db29c3..8ea0b3c8d6ff2 100644 --- a/src/repr/src/adt/timestamp.rs +++ b/src/repr/src/adt/timestamp.rs @@ -184,6 +184,14 @@ pub trait DateLike: chrono::Datelike { naive_date.and_utc().timestamp() } + /// The year in SQL's numbering, where 1 BC is year -1 and there is no + /// year 0. Chrono's `year` uses astronomical numbering, where 1 BC is + /// year 0. + fn extract_year(&self) -> i32 { + let year = self.year(); + if year <= 0 { year - 1 } else { year } + } + fn millennium(&self) -> i32 { (self.year() + if self.year() > 0 { 999 } else { -1_000 }) / 1_000 } diff --git a/test/sqllogictest/dates-times.slt b/test/sqllogictest/dates-times.slt index 2596594079e11..430f8b85d1837 100644 --- a/test/sqllogictest/dates-times.slt +++ b/test/sqllogictest/dates-times.slt @@ -1752,3 +1752,53 @@ query T SELECT timestamp::timestamp from t ORDER BY t.timestamp; ---- 2012-03-05 00:00:00 + +# Regression tests for SQL-467. + +# BC years in extract are numbered with 1 BC as year -1, there is no year 0. +query RRR +SELECT extract(year FROM DATE '2020-08-11 BC'), + extract(year FROM TIMESTAMP '2020-08-11 12:00:00 BC'), + extract(year FROM TIMESTAMPTZ '2020-08-11 12:00:00+00 BC') +---- +-2020 -2020 -2020 + +query RR +SELECT extract(year FROM DATE '0001-08-11 BC'), extract(year FROM DATE '0001-08-11') +---- +-1 1 + +# make_timestamp rejects year 0 and treats negative years as BC. +query error date out of range +SELECT make_timestamp(0, 7, 15, 12, 30, 15) + +query T +SELECT make_timestamp(-1, 7, 15, 12, 30, 15) +---- +0001-07-15 12:30:15 BC + +query T +SELECT make_timestamp(-2020, 8, 11, 12, 30, 15) +---- +2020-08-11 12:30:15 BC + +# make_timestamp allows exactly 24:00:00 and 60 seconds, which roll over. +query T +SELECT make_timestamp(1999, 12, 31, 24, 0, 0) +---- +2000-01-01 00:00:00 + +query T +SELECT make_timestamp(2013, 7, 15, 12, 30, 60) +---- +2013-07-15 12:31:00 + +query error timestamp out of range +SELECT make_timestamp(1999, 12, 31, 24, 0, 1) + +# Invalid fields are errors, not NULL. +query error date out of range +SELECT make_timestamp(2020, 13, 1, 0, 0, 0) + +query error timestamp out of range +SELECT make_timestamp(2020, 1, 1, 12, 60, 0) diff --git a/test/sqllogictest/not-null-propagation.slt b/test/sqllogictest/not-null-propagation.slt index ecfb92af890cb..72f7b4ca6e92d 100644 --- a/test/sqllogictest/not-null-propagation.slt +++ b/test/sqllogictest/not-null-propagation.slt @@ -421,10 +421,27 @@ EOF # VARIADIC FUNCTIONS that introduce nulls -query BBBBBBB -SELECT COALESCE(NULLIF('a', 'a')) IS NULL, GREATEST(NULLIF('a', 'a')) IS NULL, LEAST(NULLIF('a', 'a')) IS NULL, MAKE_TIMESTAMP(2023, 1, 1, 0, 0, 11111) IS NULL, (ARRAY[1, 2])[3] IS NULL, (LIST[1, 2])[3] IS NULL, REGEXP_MATCH('a', 'b') IS NULL; +query BBBBBB +SELECT COALESCE(NULLIF('a', 'a')) IS NULL, GREATEST(NULLIF('a', 'a')) IS NULL, LEAST(NULLIF('a', 'a')) IS NULL, (ARRAY[1, 2])[3] IS NULL, (LIST[1, 2])[3] IS NULL, REGEXP_MATCH('a', 'b') IS NULL; ---- -true true true true true true true +true true true true true true + +# MAKE_TIMESTAMP errors on invalid fields rather than returning NULL, but its +# return type stays nullable, so it still propagates as NULL-able. + +query T multiline +EXPLAIN OPTIMIZED PLAN WITH(types, no fast path, humanized expressions) AS VERBOSE TEXT FOR SELECT MAKE_TIMESTAMP(col_not_null, 1, 1, 0, 0, 0) FROM int_table; +---- +Explained Query: + Project (#2) // { types: "(r_timestamp?)" } + Map (makets(integer_to_bigint(#1{col_not_null}), 1, 1, 0, 0, 0)) // { types: "(r_int32?, r_int32, r_timestamp?)" } + ReadStorage materialize.public.int_table // { types: "(r_int32?, r_int32)" } + +Source materialize.public.int_table + +Target cluster: quickstart + +EOF # BINARY FUNCTIONS that introduce nulls # MapGetValue and ListLengthMax not covered From 31479921ed2281892ae053a893995485f078a271 Mon Sep 17 00:00:00 2001 From: Aljoscha Krettek Date: Fri, 24 Jul 2026 10:48:10 +0200 Subject: [PATCH 2/6] test: cover forcing an in-flight cluster reconfiguration (#37844) Verify that reissuing a graceful cluster reconfiguration for the same target with a zero timeout cuts over to the existing target replica instead of creating a replacement. --- test/testdrive/cluster-controller.td | 67 ++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/test/testdrive/cluster-controller.td b/test/testdrive/cluster-controller.td index a84a0d5c69e80..8a50999d7c2dd 100644 --- a/test/testdrive/cluster-controller.td +++ b/test/testdrive/cluster-controller.td @@ -330,6 +330,73 @@ scale=1,workers=4 > DROP CLUSTER cc_forced CASCADE +# A second ALTER to the same target can force an in-flight reconfiguration to +# cut over without replacing the target replica. +> CREATE CLUSTER cc_force_existing (SIZE 'scale=1,workers=1', REPLICATION FACTOR 1) + +> CREATE TABLE cc_force_existing_t (id int) + +> INSERT INTO cc_force_existing_t VALUES (1) + +> CREATE MATERIALIZED VIEW cc_force_existing_slow IN CLUSTER cc_force_existing AS + SELECT mz_unsafe.mz_sleep(id * 3600) AS s FROM cc_force_existing_t + +> ALTER CLUSTER cc_force_existing SET (SIZE 'scale=1,workers=4') WITH (WAIT UNTIL READY (TIMEOUT '600s', ON TIMEOUT 'ROLLBACK')) + +# The long deadline leaves both the realized and un-hydrated target replicas +# online while the reconfiguration remains in progress. +> SELECT r.size, bool_and(status.status = 'online') + FROM mz_cluster_replicas r + JOIN mz_clusters c ON c.id = r.cluster_id + JOIN mz_internal.mz_cluster_replica_statuses status ON status.replica_id = r.id + WHERE c.name = 'cc_force_existing' + GROUP BY r.size +scale=1,workers=1 true +scale=1,workers=4 true + +> SELECT recon.status + FROM mz_internal.mz_cluster_reconfigurations recon + JOIN mz_clusters c ON c.id = recon.cluster_id + WHERE c.name = 'cc_force_existing' +in-progress + +$ set-from-sql var=cc_force_existing_target_id +SELECT r.id::text +FROM mz_cluster_replicas r +JOIN mz_clusters c ON c.id = r.cluster_id +WHERE c.name = 'cc_force_existing' AND r.size = 'scale=1,workers=4' + +# Replace only the deadline and timeout action. The existing target shape and +# replica are reused, and the elapsed deadline forces cut-over on the next tick. +> ALTER CLUSTER cc_force_existing SET (SIZE 'scale=1,workers=4') WITH (WAIT UNTIL READY (TIMEOUT '0s', ON TIMEOUT 'COMMIT')) + +> SELECT r.id::text = '${cc_force_existing_target_id}', r.size + FROM mz_cluster_replicas r + JOIN mz_clusters c ON c.id = r.cluster_id + WHERE c.name = 'cc_force_existing' +true scale=1,workers=4 + +> SELECT row_number() OVER (ORDER BY id), details->>'transition', details->>'forced' + FROM mz_catalog.mz_audit_events + WHERE event_type = 'alter' + AND object_type = 'cluster' + AND details->>'cluster_name' = 'cc_force_existing' + AND details->>'transition' IS NOT NULL +1 started +2 started +3 finalized true + +# Only the first ALTER created a target replica. +> SELECT count(*) + FROM mz_catalog.mz_audit_events + WHERE event_type = 'create' + AND object_type = 'cluster-replica' + AND details->>'cluster_name' = 'cc_force_existing' + AND details->>'reason' = 'reconfiguration' +1 + +> DROP CLUSTER cc_force_existing CASCADE + $ postgres-execute connection=postgres://mz_system@${testdrive.materialize-internal-sql-addr} ALTER SYSTEM SET unsafe_enable_unstable_dependencies = false From fe90df32c45ae46543cf4d98738c1c97bbcd4d5f Mon Sep 17 00:00:00 2001 From: Dennis Felsing Date: Fri, 24 Jul 2026 11:04:46 +0200 Subject: [PATCH 3/6] sql: keep fields above interval range qualifiers (#37852) Interval range qualifiers like HOUR TO MINUTE now only resolve ambiguous parts and discard fields below the range's low end, matching PostgreSQL. Previously fields above the range's high end were dropped, and a leading day component caused "a:b" values to be read as hours:minutes even under MINUTE TO SECOND: SELECT interval '1 2:03' hour to minute; -- was 02:03:00, now 1 day 02:03:00 SELECT interval '1 2:03' minute to second; -- was 00:03:00, now 1 day 00:02:03 Closes: SQL-474 Co-authored-by: Claude Fable 5 --- src/repr/src/adt/datetime.rs | 5 +- src/repr/src/adt/interval.rs | 114 ----------------------- src/sql-parser/src/ast/defs/value.rs | 5 +- src/sql/src/plan/literal.rs | 4 +- test/sqllogictest/cockroach/interval.slt | 48 +++------- test/sqllogictest/interval.slt | 25 +++-- 6 files changed, 43 insertions(+), 158 deletions(-) diff --git a/src/repr/src/adt/datetime.rs b/src/repr/src/adt/datetime.rs index 12232874f325e..79d78fc7d1294 100644 --- a/src/repr/src/adt/datetime.rs +++ b/src/repr/src/adt/datetime.rs @@ -735,7 +735,10 @@ impl ParsedDateTime { if fmt.is_none() { fmt = match value_parts.pop_front() { Some(next_part) => { - match determine_format_w_datetimefield(next_part.clone(), None)? { + match determine_format_w_datetimefield( + next_part.clone(), + leading_time_precision, + )? { Some(TimePartFormat::SqlStandard(f)) => { match f { // Do not capture this token because expression diff --git a/src/repr/src/adt/interval.rs b/src/repr/src/adt/interval.rs index 65e775b5914d4..358ce9e20e2e8 100644 --- a/src/repr/src/adt/interval.rs +++ b/src/repr/src/adt/interval.rs @@ -448,27 +448,6 @@ impl Interval { Ok(Duration::from_micros(micros)) } - /// Truncate the "head" of the interval, removing all time units greater than `f`. - pub fn truncate_high_fields(&mut self, f: DateTimeField) { - match f { - DateTimeField::Year => {} - DateTimeField::Month => self.months %= 12, - DateTimeField::Day => self.months = 0, - DateTimeField::Hour | DateTimeField::Minute | DateTimeField::Second => { - self.months = 0; - self.days = 0; - self.micros %= f.next_largest().micros_multiplier() - } - DateTimeField::Millennium - | DateTimeField::Century - | DateTimeField::Decade - | DateTimeField::Milliseconds - | DateTimeField::Microseconds => { - unreachable!("Cannot truncate interval by {f}"); - } - } - } - /// Truncate the "tail" of the interval, removing all time units less than `f`. /// # Arguments /// - `f`: Round the interval down to the specified time unit. @@ -1293,99 +1272,6 @@ mod test { } } - #[mz_ore::test] - fn test_interval_value_truncate_high_fields() { - use DateTimeField::*; - - // (month, day, microsecond) tuples - let mut test_cases = [ - ( - Year, - ( - 321, - 7, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - ( - 321, - 7, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - ), - ( - Month, - ( - 321, - 7, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - ( - 9, - 7, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - ), - ( - Day, - ( - 321, - 7, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - ( - 0, - 7, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - ), - ( - Hour, - ( - 321, - 7, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - ( - 0, - 0, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - ), - ( - Minute, - ( - 321, - 7, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - (0, 0, (45 * 60 * 1_000_000) + (21 * 1_000_000)), - ), - ( - Second, - ( - 321, - 7, - (13 * 60 * 60 * 1_000_000) + (45 * 60 * 1_000_000) + (21 * 1_000_000), - ), - (0, 0, 21 * 1_000_000), - ), - ]; - - for test in test_cases.iter_mut() { - let mut i = Interval::new((test.1).0, (test.1).1, (test.1).2); - let j = Interval::new((test.2).0, (test.2).1, (test.2).2); - - i.truncate_high_fields(test.0); - - if i != j { - panic!( - "test_interval_value_truncate_high_fields failed on {} \n actual: {:?} \n expected: {:?}", - test.0, i, j - ); - } - } - } - #[mz_ore::test] fn test_convert_date_time_unit() { assert_eq!( diff --git a/src/sql-parser/src/ast/defs/value.rs b/src/sql-parser/src/ast/defs/value.rs index 348ebb7ae0826..2556d7b14c56b 100644 --- a/src/sql-parser/src/ast/defs/value.rs +++ b/src/sql-parser/src/ast/defs/value.rs @@ -249,8 +249,9 @@ impl FromStr for DateTimeField { pub struct IntervalValue { /// The raw `[value]` that was present in `INTERVAL '[value]'` pub value: String, - /// The most significant DateTimeField to propagate to Interval in - /// compute_interval. + /// The high end of the range qualifier, e.g. HOUR in `INTERVAL '...' + /// HOUR TO MINUTE`. Only disambiguates parsing of ambiguous time-like + /// parts of `value`. Fields above it are kept, as in PostgreSQL. pub precision_high: DateTimeField, /// The least significant DateTimeField to propagate to Interval in /// compute_interval. diff --git a/src/sql/src/plan/literal.rs b/src/sql/src/plan/literal.rs index 25eab9c41c8fa..50e8dc9327281 100644 --- a/src/sql/src/plan/literal.rs +++ b/src/sql/src/plan/literal.rs @@ -27,7 +27,9 @@ pub fn plan_interval(iv: &IntervalValue) -> Result { }, parser_datetimefield_to_adt(iv.precision_low), )?; - i.truncate_high_fields(parser_datetimefield_to_adt(iv.precision_high)); + // Like PostgreSQL, the range qualifier only disambiguates parsing and + // discards fields below precision_low. Fields above precision_high are + // kept as written. i.truncate_low_fields( parser_datetimefield_to_adt(iv.precision_low), iv.fsec_max_precision, diff --git a/test/sqllogictest/cockroach/interval.slt b/test/sqllogictest/cockroach/interval.slt index b88b9aeb90939..9ec3394f3a857 100644 --- a/test/sqllogictest/cockroach/interval.slt +++ b/test/sqllogictest/cockroach/interval.slt @@ -194,77 +194,61 @@ SELECT interval '1 2:03:04' day to second query error invalid input syntax for type interval: Cannot determine format of all parts\. Add explicit time components, e\.g\. INTERVAL '1 day' or INTERVAL '1' DAY: "1 2" SELECT interval '1 2' hour to minute -# SQL-474 (https://linear.app/materializeinc/issue/SQL-474): Materialize drops -# interval fields to the left of the range qualifier's leading unit, even when -# they were written explicitly. PG and CockroachDB keep every explicitly-written -# field and only use the qualifier to resolve ambiguity. Some inputs also change -# magnitude. The commented "PG/CRDB:" lines are the outputs to restore on fix. - -# SQL-474 PG/CRDB: 1 day 02:03:00 query T SELECT interval '1 2:03' hour to minute ---- -02:03:00 +1 day 02:03:00 -# SQL-474 PG/CRDB: 1 day 02:03:00 query T SELECT interval '1 2:03:04' hour to minute ---- -02:03:00 +1 day 02:03:00 query error invalid input syntax for type interval: Cannot determine format of all parts\. Add explicit time components, e\.g\. INTERVAL '1 day' or INTERVAL '1' DAY: "1 2" SELECT interval '1 2' hour to second -# SQL-474 PG/CRDB: 1 day 02:03:00 query T SELECT interval '1 2:03' hour to second ---- -02:03:00 +1 day 02:03:00 -# SQL-474 PG/CRDB: 1 day 02:03:04 query T SELECT interval '1 2:03:04' hour to second ---- -02:03:04 +1 day 02:03:04 query error invalid input syntax for type interval: Cannot determine format of all parts\. Add explicit time components, e\.g\. INTERVAL '1 day' or INTERVAL '1' DAY: "1 2" SELECT interval '1 2' minute to second -# SQL-474 PG/CRDB: 1 day 00:02:03 (MZ also mis-scales "2:03" as MM instead of MM:SS) query T SELECT interval '1 2:03' minute to second ---- -00:03:00 +1 day 00:02:03 -# SQL-474 PG/CRDB: 1 day 02:03:04 query T SELECT interval '1 2:03:04' minute to second ---- -00:03:04 +1 day 02:03:04 -# SQL-474 PG/CRDB: 1 day 00:02:03 query T SELECT interval '1 +2:03' minute to second ---- -00:03:00 +1 day 00:02:03 -# SQL-474 PG/CRDB: 1 day 02:03:04 query T SELECT interval '1 +2:03:04' minute to second ---- -00:03:04 +1 day 02:03:04 -# SQL-474 PG/CRDB: 1 day -00:02:03 query T SELECT interval '1 -2:03' minute to second ---- --00:03:00 +1 day -00:02:03 -# SQL-474 PG/CRDB: 1 day -02:03:04 query T SELECT interval '1 -2:03:04' minute to second ---- --00:03:04 +1 day -02:03:04 # Not supported by Materialize. onlyif cockroach @@ -334,32 +318,28 @@ SELECT interval '1 2:03:04.5678' day to second(2) query error invalid input syntax for type interval: Cannot determine format of all parts\. Add explicit time components, e\.g\. INTERVAL '1 day' or INTERVAL '1' DAY: "1 2\.345" SELECT interval '1 2.345' hour to second(2) -# SQL-474 PG/CRDB: 1 day 00:02:03.46 query T SELECT interval '1 2:03.45678' hour to second(2) ---- -00:02:03.46 +1 day 00:02:03.46 -# SQL-474 PG/CRDB: 1 day 02:03:04.57 query T SELECT interval '1 2:03:04.5678' hour to second(2) ---- -02:03:04.57 +1 day 02:03:04.57 query error invalid input syntax for type interval: Cannot determine format of all parts\. Add explicit time components, e\.g\. INTERVAL '1 day' or INTERVAL '1' DAY: "1 2\.3456" SELECT interval '1 2.3456' minute to second(2) -# SQL-474 PG/CRDB: 1 day 00:02:03.57 query T SELECT interval '1 2:03.5678' minute to second(2) ---- -00:02:03.57 +1 day 00:02:03.57 -# SQL-474 PG/CRDB: 1 day 02:03:04.57 query T SELECT interval '1 2:03:04.5678' minute to second(2) ---- -00:03:04.57 +1 day 02:03:04.57 # Extra regression tests found when fixing this bug. subtest regression_43074 diff --git a/test/sqllogictest/interval.slt b/test/sqllogictest/interval.slt index 0e1805f845267..97643e6b06fd8 100644 --- a/test/sqllogictest/interval.slt +++ b/test/sqllogictest/interval.slt @@ -116,16 +116,17 @@ SELECT INTERVAL '1-2 3 4:5:6.7' SECOND; ---- 1 year 2 months 3 days 04:05:06.7 -# Treat trailing DateTimeFields as range of components to process. +# Range qualifiers discard fields less significant than the range's low end, +# but keep more significant fields, matching PostgreSQL. query T SELECT INTERVAL '1-2 3 4:5:6.7' MONTH TO MINUTE; ---- -2 months 3 days 04:05:00 +1 year 2 months 3 days 04:05:00 query T SELECT INTERVAL '1-2 3 4:5:6.7' DAY TO HOUR; ---- -3 days 04:00:00 +1 year 2 months 3 days 04:00:00 query T SELECT INTERVAL '12:34'; @@ -195,7 +196,7 @@ SELECT INTERVAL '12:34:56' HOUR TO SECOND; query T SELECT INTERVAL '12:34:56' MINUTE TO SECOND; ---- -00:34:56 +12:34:56 query T SELECT INTERVAL '12:34' DAY TO SECOND; @@ -207,6 +208,18 @@ SELECT INTERVAL '12:34' MONTH TO SECOND; ---- 12:34:00 +# Regression tests for SQL-474: fields above the range qualifier are kept, and +# MINUTE TO SECOND reads ambiguous a:b values as minutes:seconds. +query T +SELECT INTERVAL '1 2:03' HOUR TO MINUTE; +---- +1 day 02:03:00 + +query T +SELECT INTERVAL '1 2:03' MINUTE TO SECOND; +---- +1 day 00:02:03 + # Disallow ranges where trailing element is greater than lead. statement error SELECT INTERVAL '1-2 3 4:5:6.7' MINUTE TO MONTH; @@ -522,7 +535,7 @@ SELECT INTERVAL '1.5555 month 2 3:4:5.6' HOUR; query T SELECT INTERVAL '1.5555 month 2 3:4:5.6' DAY TO HOUR; ---- -18 days 19:00:00 +1 month 18 days 19:00:00 # Only allow disambiguation if ambiguous element # is final element of interval string. @@ -602,7 +615,7 @@ SELECT INTERVAL '-1.555555555 years 2.555555555 months -3.555555555 days 4.55555 query T SELECT INTERVAL '1:2:31.23456789' MINUTE TO SECOND(2); ---- -00:02:31.23 +01:02:31.23 # Do not allow more than 9 places of precision query T From 669b67e3d1f56ffce2b0cd986f5b231484057329 Mon Sep 17 00:00:00 2001 From: Dennis Felsing Date: Fri, 24 Jul 2026 11:07:02 +0200 Subject: [PATCH 4/6] repr: Honor DateStyle MDY and two-digit-year windowing in date parsing (#37838) Interpret separated all-numeric date literals the way PostgreSQL does under the pinned DateStyle 'ISO, MDY': a one- or two-digit leading field starts a month-day-year date, and a two-digit year is windowed into 1970..=2069. A leading field of three or more digits keeps the year-month-day interpretation. This applies to date, timestamp and timestamptz parsing at the SQL layer. SELECT date '01/02/03'; -- now 2003-01-02, was 0001-02-03 SELECT date '99-01-08'; -- now an error (month 99), was 0099-01-08 Storage source casts stay on new frozen legacy parse functions (parse_date_legacy and friends) so their evaluation is unchanged, per the stability contract in mz_storage_types::sources::casts. Closes: SQL-461 Co-authored-by: Claude Fable 5 --- src/repr/src/adt/datetime.rs | 127 ++++++++++++++++++++++- src/repr/src/strconv.rs | 134 +++++++++++++++++++++++-- src/repr/src/timestamp.rs | 24 ++++- src/storage-types/src/sources/casts.rs | 31 ++++-- test/sqllogictest/dates-times.slt | 55 ++++++++++ test/sqllogictest/timestamptz.slt | 2 +- 6 files changed, 355 insertions(+), 18 deletions(-) diff --git a/src/repr/src/adt/datetime.rs b/src/repr/src/adt/datetime.rs index 79d78fc7d1294..72aea07e75dcd 100644 --- a/src/repr/src/adt/datetime.rs +++ b/src/repr/src/adt/datetime.rs @@ -816,12 +816,13 @@ impl ParsedDateTime { pub fn build_parsed_datetime_timestamp( value: &str, era: CalendarEra, + order: DateOrder, ) -> Result { let mut pdt = ParsedDateTime::default(); let mut ts_actual = tokenize_time_str(value)?; - fill_pdt_date(&mut pdt, &mut ts_actual)?; + fill_pdt_date(&mut pdt, &mut ts_actual, order, era)?; if let CalendarEra::BC = era { pdt.year = pdt.year.map(|mut y| { @@ -1083,6 +1084,8 @@ impl ParsedDateTime { fn fill_pdt_date( pdt: &mut ParsedDateTime, actual: &mut VecDeque, + order: DateOrder, + era: CalendarEra, ) -> Result<(), String> { use TimeStrToken::*; @@ -1127,6 +1130,26 @@ fn fill_pdt_date( _ => (), } + // PostgreSQL under the pinned `DateStyle = ISO, MDY` reads a separated + // date whose leading field has one or two digits as month-day-year, and + // only a leading field of three or more digits as year-month-day. The + // formats below fill fields positionally as year-month-day, so record the + // digit counts up front and reorder the fields after a successful fill. + let mdy = match (order, actual.front()) { + (DateOrder::Mdy, Some(&Num(_, digits))) => digits <= 2, + _ => false, + }; + // Digit count of the third numeric field, the year in a month-day-year + // date, which decides two-digit-year windowing. + let year_digits = actual + .iter() + .take_while(|t| matches!(t, Num(..) | Dash | Delim)) + .filter_map(|t| match t { + Num(_, digits) => Some(*digits), + _ => None, + }) + .nth(2); + let valid_formats = [ [ Num(0, 1), // year @@ -1156,6 +1179,22 @@ fn fill_pdt_date( for expected in valid_formats { match fill_pdt_from_tokens(pdt, actual, &expected, DateTimeField::Year, 1) { Ok(()) => { + // In a month-day-year date the positional fill above put the + // month in `year`, the day in `month`, and the year in `day`. + if mdy { + if let (Some(month), Some(day), Some(mut year)) = (pdt.year, pdt.month, pdt.day) + { + // Window two-digit years into 1970..=2069, as + // PostgreSQL does. Explicit BC years are never + // windowed. + if era == CalendarEra::AD && year_digits.is_some_and(|d| d <= 2) { + year.unit += if year.unit < 70 { 2000 } else { 1900 }; + } + pdt.year = Some(year); + pdt.month = Some(month); + pdt.day = Some(day); + } + } return Ok(()); } Err(_) => { @@ -1813,12 +1852,25 @@ pub(crate) fn tokenize_time_str(value: &str) -> Result, S Ok(toks) } -#[derive(Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum CalendarEra { BC, AD, } +/// Determines how ambiguous all-numeric dates like `01/02/03` are interpreted. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum DateOrder { + /// PostgreSQL semantics under the pinned `DateStyle = ISO, MDY`: a date + /// whose leading field has one or two digits is month-day-year, and a + /// two-digit year is windowed into 1970..=2069. + Mdy, + /// Always year-month-day, with no year windowing. Frozen behavior for + /// storage source casts, which must not change across releases (see the + /// stability contract in `mz_storage_types::sources::casts`). + LegacyYmd, +} + /// Takes a 'date timezone' 'date time timezone' string and splits it into 'date /// {time}' and 'timezone' components then checks for a 'CalenderEra' defaulting /// to 'AD'. @@ -2842,10 +2894,79 @@ mod tests { ..Default::default() }, ); + // A one- or two-digit leading field starts a month-day-year date, and + // a two-digit year is windowed into 1970..=2069. + run_test_build_parsed_datetime_timestamp( + "01/02/03", + ParsedDateTime { + year: Some(DateTimeFieldValue::new(2003, 0)), + month: Some(DateTimeFieldValue::new(1, 0)), + day: Some(DateTimeFieldValue::new(2, 0)), + ..Default::default() + }, + ); + run_test_build_parsed_datetime_timestamp( + "01-02-70 3:4:5.6", + ParsedDateTime { + year: Some(DateTimeFieldValue::new(1970, 0)), + month: Some(DateTimeFieldValue::new(1, 0)), + day: Some(DateTimeFieldValue::new(2, 0)), + hour: Some(DateTimeFieldValue::new(3, 0)), + minute: Some(DateTimeFieldValue::new(4, 0)), + second: Some(DateTimeFieldValue::new(5, 600_000_000)), + ..Default::default() + }, + ); + // A four-digit year in month-day-year position is not windowed. + run_test_build_parsed_datetime_timestamp( + "1/2/1999", + ParsedDateTime { + year: Some(DateTimeFieldValue::new(1999, 0)), + month: Some(DateTimeFieldValue::new(1, 0)), + day: Some(DateTimeFieldValue::new(2, 0)), + ..Default::default() + }, + ); + // BC years are never windowed. + assert_eq!( + ParsedDateTime::build_parsed_datetime_timestamp( + "01/02/03", + CalendarEra::BC, + DateOrder::Mdy, + ) + .unwrap(), + ParsedDateTime { + year: Some(DateTimeFieldValue::new(-3, 0)), + month: Some(DateTimeFieldValue::new(1, 0)), + day: Some(DateTimeFieldValue::new(2, 0)), + ..Default::default() + }, + ); + // The legacy order keeps the frozen year-month-day interpretation + // with no windowing. + assert_eq!( + ParsedDateTime::build_parsed_datetime_timestamp( + "01/02/03", + CalendarEra::AD, + DateOrder::LegacyYmd, + ) + .unwrap(), + ParsedDateTime { + year: Some(DateTimeFieldValue::new(1, 0)), + month: Some(DateTimeFieldValue::new(2, 0)), + day: Some(DateTimeFieldValue::new(3, 0)), + ..Default::default() + }, + ); fn run_test_build_parsed_datetime_timestamp(test: &str, res: ParsedDateTime) { assert_eq!( - ParsedDateTime::build_parsed_datetime_timestamp(test, CalendarEra::AD).unwrap(), + ParsedDateTime::build_parsed_datetime_timestamp( + test, + CalendarEra::AD, + DateOrder::Mdy + ) + .unwrap(), res ); } diff --git a/src/repr/src/strconv.rs b/src/repr/src/strconv.rs index 84023a9a9fa36..4868f2bf26f4b 100644 --- a/src/repr/src/strconv.rs +++ b/src/repr/src/strconv.rs @@ -52,7 +52,7 @@ use uuid::Uuid; use crate::adt::array::ArrayDimension; use crate::adt::date::Date; -use crate::adt::datetime::{self, DateTimeField, ParsedDateTime}; +use crate::adt::datetime::{self, DateOrder, DateTimeField, ParsedDateTime}; use crate::adt::interval::Interval; use crate::adt::jsonb::{Jsonb, JsonbRef}; use crate::adt::mz_acl_item::{AclItem, MzAclItem}; @@ -225,6 +225,18 @@ pub fn parse_mz_timestamp(s: &str) -> Result { .map_err(|e| ParseError::invalid_input_syntax("mz_timestamp", s).with_details(e)) } +/// Parses an `mz_timestamp` from `s` with the frozen legacy date parsing in +/// its timestamp fallback. +/// +/// NOTE: This exists solely to keep the storage source cast +/// `CastStringToMzTimestamp` evaluation-stable across releases (see the +/// stability contract in `mz_storage_types::sources::casts`). Use +/// [`parse_mz_timestamp`] everywhere else. +pub fn parse_mz_timestamp_legacy(s: &str) -> Result { + crate::Timestamp::from_str_legacy(s.trim()) + .map_err(|e| ParseError::invalid_input_syntax("mz_timestamp", s).with_details(e)) +} + /// Writes an `mz_timestamp` to `buf`. pub fn format_mz_timestamp(buf: &mut F, u: crate::Timestamp) -> Nestable where @@ -399,7 +411,10 @@ where ///