diff --git a/e2e_test/batch/types/interval.slt.part b/e2e_test/batch/types/interval.slt.part index 73bd7189941b..bd8972d3167e 100644 --- a/e2e_test/batch/types/interval.slt.part +++ b/e2e_test/batch/types/interval.slt.part @@ -120,43 +120,33 @@ select '1 mons 1 days 00:00:00.0000001'::INTERVAL; ---- 1 mon 1 day -# Tests moved from regress tests due to not matching exactly. - -# In mixed sign intervals, PostgreSQL displays positive sign after negative -# (e.g. `-1 mons +1 day`) while we display it without sign -# (e.g. `-1 mons 1 day`). -# But the main purpose of this test case is ordering of large values. - -statement ok -CREATE TABLE INTERVAL_TBL_OF (f1 interval); - -statement ok -INSERT INTO INTERVAL_TBL_OF (f1) VALUES - ('2147483647 days 2147483647 months'), - ('2147483647 days -2147483648 months'), - ('1 year'), - ('-2147483648 days 2147483647 months'), - ('-2147483648 days -2147483648 months'); - -statement ok -FLUSH; - -query TT -SELECT r1.*, r2.* - FROM INTERVAL_TBL_OF r1, INTERVAL_TBL_OF r2 - WHERE r1.f1 > r2.f1 - ORDER BY r1.f1, r2.f1; ----- --178956970 years -8 mons 2147483647 days -178956970 years -8 mons -2147483648 days -1 year -178956970 years -8 mons -2147483648 days -1 year -178956970 years -8 mons 2147483647 days -178956970 years 7 mons -2147483648 days -178956970 years -8 mons -2147483648 days -178956970 years 7 mons -2147483648 days -178956970 years -8 mons 2147483647 days -178956970 years 7 mons -2147483648 days 1 year -178956970 years 7 mons 2147483647 days -178956970 years -8 mons -2147483648 days -178956970 years 7 mons 2147483647 days -178956970 years -8 mons 2147483647 days -178956970 years 7 mons 2147483647 days 1 year -178956970 years 7 mons 2147483647 days 178956970 years 7 mons -2147483648 days - -statement ok -DROP TABLE INTERVAL_TBL_OF; +# parsing large values + +query T +select '2562047788:00:54.775807'::interval; +---- +2562047788:00:54.775807 + +statement error +select '2562047788:00:54.775808'::interval; + +query T +select '4 years 2147483599 mon'::interval; +---- +178956970 years 7 mons + +statement error +select '4 years 2147483600 mon'::interval; + +query T +select '-2562047788:00:54.775807'::interval; +---- +-2562047788:00:54.775807 + +query T +select '-2562047788:00:54.775808'::interval; +---- +-2562047788:00:54.775808 + +statement error +select '-2562047788:00:54.775809'::interval; diff --git a/src/common/src/types/interval.rs b/src/common/src/types/interval.rs index 0dfe356caaa1..8bbe3001a0ab 100644 --- a/src/common/src/types/interval.rs +++ b/src/common/src/types/interval.rs @@ -813,42 +813,45 @@ impl Display for IntervalUnit { let months = self.months % 12; let days = self.days; let mut space = false; - let mut write = |arg: std::fmt::Arguments<'_>| { + let mut following_neg = false; + let mut write_i32 = |arg: i32, unit: &str| -> std::fmt::Result { + if arg == 0 { + return Ok(()); + } if space { write!(f, " ")?; } - write!(f, "{arg}")?; + if following_neg && arg > 0 { + write!(f, "+")?; + } + write!(f, "{arg} {unit}")?; + if arg != 1 { + write!(f, "s")?; + } space = true; + following_neg = arg < 0; Ok(()) }; - if years == 1 { - write(format_args!("{years} year"))?; - } else if years != 0 { - write(format_args!("{years} years"))?; - } - if months == 1 { - write(format_args!("{months} mon"))?; - } else if months != 0 { - write(format_args!("{months} mons"))?; - } - if days == 1 { - write(format_args!("{days} day"))?; - } else if days != 0 { - write(format_args!("{days} days"))?; - } + write_i32(years, "year")?; + write_i32(months, "mon")?; + write_i32(days, "day")?; if self.usecs != 0 || self.months == 0 && self.days == 0 { - let usecs = self.usecs.abs(); - let ms = usecs / 1000; - let hours = ms / 1000 / 3600; - let minutes = (ms / 1000 / 60) % 60; - let seconds = ms % 60000 / 1000; - let secs_fract = usecs % USECS_PER_SEC; - - if self.usecs < 0 { - write(format_args!("-{hours:0>2}:{minutes:0>2}:{seconds:0>2}"))?; - } else { - write(format_args!("{hours:0>2}:{minutes:0>2}:{seconds:0>2}"))?; + // `abs` on `self.usecs == i64::MIN` would overflow, so we divide first then abs + let secs_fract = (self.usecs % USECS_PER_SEC).abs(); + let total_secs = (self.usecs / USECS_PER_SEC).abs(); + let hours = total_secs / 3600; + let minutes = (total_secs / 60) % 60; + let seconds = total_secs % 60; + + if space { + write!(f, " ")?; + } + if following_neg && self.usecs > 0 { + write!(f, "+")?; + } else if self.usecs < 0 { + write!(f, "-")?; } + write!(f, "{hours:0>2}:{minutes:0>2}:{seconds:0>2}")?; if secs_fract != 0 { let mut buf = [0u8; 7]; write!(buf.as_mut_slice(), ".{:06}", secs_fract).unwrap(); @@ -953,7 +956,7 @@ fn parse_interval(s: &str) -> Result> { let mut hour_min_sec = Vec::new(); for (i, c) in s.chars().enumerate() { match c { - '-' => { + '-' | '+' => { num_buf.push(c); } '.' => { @@ -1001,7 +1004,9 @@ fn parse_interval(s: &str) -> Result> { convert_digit(&mut num_buf, &mut tokens)?; } convert_unit(&mut char_buf, &mut tokens)?; - convert_hms(&mut hour_min_sec, &mut tokens)?; + convert_hms(&mut hour_min_sec, &mut tokens).ok_or_else(|| { + ErrorCode::InvalidInputSyntax(format!("Invalid interval: {:?}", hour_min_sec)) + })?; Ok(tokens) } @@ -1037,34 +1042,49 @@ fn convert_unit(c: &mut String, t: &mut Vec) -> Result<()> { /// [`TimeStrToken::Num(1)`, `TimeStrToken::TimeUnit(DateTimeField::Hour)`, /// `TimeStrToken::Num(2)`, `TimeStrToken::TimeUnit(DateTimeField::Minute)`, /// `TimeStrToken::Second("3")`, `TimeStrToken::TimeUnit(DateTimeField::Second)`] -fn convert_hms(c: &mut Vec, t: &mut Vec) -> Result<()> { +fn convert_hms(c: &mut Vec, t: &mut Vec) -> Option<()> { if c.len() > 3 { - return Err(ErrorCode::InvalidInputSyntax(format!("Invalid interval: {:?}", c)).into()); + return None; } + const HOUR: usize = 0; + const MINUTE: usize = 1; + const SECOND: usize = 2; + let mut is_neg = false; for (i, s) in c.iter().enumerate() { match i { - 0 => { - t.push(TimeStrToken::Num(s.parse().map_err(|_| { - ErrorCode::InternalError(format!("Invalid interval: {}", c[0])) - })?)); + HOUR => { + let v = s.parse().ok()?; + is_neg = v < 0; + t.push(TimeStrToken::Num(v)); t.push(TimeStrToken::TimeUnit(DateTimeField::Hour)) } - 1 => { - t.push(TimeStrToken::Num(s.parse().map_err(|_| { - ErrorCode::InternalError(format!("Invalid interval: {}", c[0])) - })?)); + MINUTE => { + let mut v: i64 = s.parse().ok()?; + if !(0..60).contains(&v) { + return None; + } + if is_neg { + v = v.checked_neg()?; + } + t.push(TimeStrToken::Num(v)); t.push(TimeStrToken::TimeUnit(DateTimeField::Minute)) } - 2 => { - t.push(TimeStrToken::Second(s.parse().map_err(|_| { - ErrorCode::InternalError(format!("Invalid interval: {}", c[0])) - })?)); + SECOND => { + let mut v: OrderedF64 = s.parse().ok()?; + // PostgreSQL allows '60.x' for seconds. + if !(0f64 <= *v && *v < 61f64) { + return None; + } + if is_neg { + v = v.checked_neg()?; + } + t.push(TimeStrToken::Second(v)); t.push(TimeStrToken::TimeUnit(DateTimeField::Second)) } _ => unreachable!(), } } - Ok(()) + Some(()) } impl IntervalUnit { @@ -1092,11 +1112,19 @@ impl IntervalUnit { (|| match leading_field { Year => { - let months = num.checked_mul(12)?; - Some(IntervalUnit::from_month_day_usec(months as i32, 0, 0)) + let months = num.checked_mul(12)?.try_into().ok()?; + Some(IntervalUnit::from_month_day_usec(months, 0, 0)) } - Month => Some(IntervalUnit::from_month_day_usec(num as i32, 0, 0)), - Day => Some(IntervalUnit::from_month_day_usec(0, num as i32, 0)), + Month => Some(IntervalUnit::from_month_day_usec( + num.try_into().ok()?, + 0, + 0, + )), + Day => Some(IntervalUnit::from_month_day_usec( + 0, + num.try_into().ok()?, + 0, + )), Hour => { let usecs = num.checked_mul(3600 * USECS_PER_SEC)?; Some(IntervalUnit::from_month_day_usec(0, 0, usecs)) @@ -1127,13 +1155,13 @@ impl IntervalUnit { while let Some(num) = token_iter.next() && let Some(interval_unit) = token_iter.next() { match (num, interval_unit) { (TimeStrToken::Num(num), TimeStrToken::TimeUnit(interval_unit)) => { - result = result + (|| match interval_unit { + result = (|| match interval_unit { Year => { - let months = num.checked_mul(12)?; - Some(IntervalUnit::from_month_day_usec(months as i32, 0, 0)) + let months = num.checked_mul(12)?.try_into().ok()?; + Some(IntervalUnit::from_month_day_usec(months, 0, 0)) } - Month => Some(IntervalUnit::from_month_day_usec(num as i32, 0, 0)), - Day => Some(IntervalUnit::from_month_day_usec(0, num as i32, 0)), + Month => Some(IntervalUnit::from_month_day_usec(num.try_into().ok()?, 0, 0)), + Day => Some(IntervalUnit::from_month_day_usec(0, num.try_into().ok()?, 0)), Hour => { let usecs = num.checked_mul(3600 * USECS_PER_SEC)?; Some(IntervalUnit::from_month_day_usec(0, 0, usecs)) @@ -1147,10 +1175,11 @@ impl IntervalUnit { Some(IntervalUnit::from_month_day_usec(0, 0, usecs)) } })() + .and_then(|rhs| result.checked_add(&rhs)) .ok_or_else(|| ErrorCode::InvalidInputSyntax(format!("Invalid interval {}.", s)))?; } (TimeStrToken::Second(second), TimeStrToken::TimeUnit(interval_unit)) => { - result = result + match interval_unit { + result = match interval_unit { Second => { // If unsatisfied precision is passed as input, we should not return None (Error). let usecs = (second.into_inner() * (USECS_PER_SEC as f64)).round() as i64; @@ -1158,6 +1187,7 @@ impl IntervalUnit { } _ => None, } + .and_then(|rhs| result.checked_add(&rhs)) .ok_or_else(|| ErrorCode::InvalidInputSyntax(format!("Invalid interval {}.", s)))?; } _ => { @@ -1275,11 +1305,11 @@ mod tests { (11 * 3600 + 45 * 60 + 14) * USECS_PER_SEC + 233 ) .to_string(), - "-1 years -2 mons 3 days 11:45:14.000233" + "-1 years -2 mons +3 days 11:45:14.000233" ); assert_eq!( IntervalUnit::from_month_day_usec(-14, 3, 0).to_string(), - "-1 years -2 mons 3 days" + "-1 years -2 mons +3 days" ); assert_eq!(IntervalUnit::default().to_string(), "00:00:00"); assert_eq!( @@ -1289,7 +1319,7 @@ mod tests { -((11 * 3600 + 45 * 60 + 14) * USECS_PER_SEC + 233) ) .to_string(), - "-1 years -2 mons 3 days -11:45:14.000233" + "-1 years -2 mons +3 days -11:45:14.000233" ); } diff --git a/src/tests/regress/data/sql/interval.sql b/src/tests/regress/data/sql/interval.sql index fdec559bac59..b0c22dcb2515 100644 --- a/src/tests/regress/data/sql/interval.sql +++ b/src/tests/regress/data/sql/interval.sql @@ -7,13 +7,13 @@ SET IntervalStyle to postgres; -- check acceptance of "time zone style" SELECT INTERVAL '01:00' AS "One hour"; ---@ SELECT INTERVAL '+02:00' AS "Two hours"; +SELECT INTERVAL '+02:00' AS "Two hours"; SELECT INTERVAL '-08:00' AS "Eight hours"; --@ SELECT INTERVAL '-1 +02:03' AS "22 hours ago..."; ---@ SELECT INTERVAL '-1 days +02:03' AS "22 hours ago..."; +SELECT INTERVAL '-1 days +02:03' AS "22 hours ago..."; --@ SELECT INTERVAL '1.5 weeks' AS "Ten days twelve hours"; --@ SELECT INTERVAL '1.5 months' AS "One month 15 days"; ---@ SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years..."; +SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years..."; CREATE TABLE INTERVAL_TBL (f1 interval); @@ -69,24 +69,24 @@ INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 days 2147483647 months'), ('-2147483648 days -2147483648 months'); -- these should fail as out-of-range ---@ INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483648 days'); ---@ INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483649 days'); ---@ INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483647 years'); ---@ INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years'); +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483648 days'); +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483649 days'); +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483647 years'); +INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years'); -- Test edge-case overflow detection in interval multiplication --@ select extract(epoch from '256 microseconds'::interval * (2^55)::float8); ---@ SELECT r1.*, r2.* ---@ FROM INTERVAL_TBL_OF r1, INTERVAL_TBL_OF r2 ---@ WHERE r1.f1 > r2.f1 ---@ ORDER BY r1.f1, r2.f1; +SELECT r1.*, r2.* + FROM INTERVAL_TBL_OF r1, INTERVAL_TBL_OF r2 + WHERE r1.f1 > r2.f1 + ORDER BY r1.f1, r2.f1; --@ CREATE INDEX ON INTERVAL_TBL_OF USING btree (f1); --@ SET enable_seqscan TO false; --@ EXPLAIN (COSTS OFF) --@ SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1; ---@ SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1; +SELECT f1 FROM INTERVAL_TBL_OF r1 ORDER BY f1; --@ RESET enable_seqscan; DROP TABLE INTERVAL_TBL_OF; @@ -101,15 +101,15 @@ DROP TABLE INTERVAL_TBL_OF; CREATE TABLE INTERVAL_MULDIV_TBL (span interval); -- COPY INTERVAL_MULDIV_TBL FROM STDIN; ---@ INSERT INTO INTERVAL_MULDIV_TBL VALUES ---@ ('41 mon 12 days 360:00'), ---@ ('-41 mon -12 days +360:00'), ---@ ('-12 days'), ---@ ('9 mon -27 days 12:34:56'), ---@ ('-3 years 482 days 76:54:32.189'), ---@ ('4 mon'), ---@ ('14 mon'), ---@ ('999 mon 999 days'); +INSERT INTO INTERVAL_MULDIV_TBL VALUES +('41 mon 12 days 360:00'), +('-41 mon -12 days +360:00'), +('-12 days'), +('9 mon -27 days 12:34:56'), +('-3 years 482 days 76:54:32.189'), +('4 mon'), +('14 mon'), +('999 mon 999 days'); --@ SELECT span * 0.3 AS product --@ FROM INTERVAL_MULDIV_TBL; @@ -284,8 +284,8 @@ SET IntervalStyle to postgres; --@ interval 'PT10:30' AS "hour minute"; -- test a couple rounding cases that changed since 8.3 w/ HAVE_INT64_TIMESTAMP. ---@ select interval '-10 mons -3 days +03:55:06.70'; ---@ select interval '1 year 2 mons 3 days 04:05:06.699999'; +select interval '-10 mons -3 days +03:55:06.70'; +select interval '1 year 2 mons 3 days 04:05:06.699999'; --@ select interval '0:0:0.7', interval '@ 0.70 secs', interval '0.7 seconds'; -- check that '30 days' equals '1 month' according to the hash function