From 57e1d74bfb374eb4334489b1a0156e032077577b Mon Sep 17 00:00:00 2001 From: Sam Hughes Date: Wed, 12 Jun 2024 14:59:39 -0700 Subject: [PATCH] fix(cubesql): Proper interval dataframe values and better formatting --- rust/cubesql/cubesql/src/compile/mod.rs | 48 +++++++++++++++++------ rust/cubesql/cubesql/src/sql/dataframe.rs | 24 ++++++++++-- rust/cubesql/pg-srv/src/encoding.rs | 36 +++++++++++++++-- 3 files changed, 90 insertions(+), 18 deletions(-) diff --git a/rust/cubesql/cubesql/src/compile/mod.rs b/rust/cubesql/cubesql/src/compile/mod.rs index eb47d91cc5b97..7605f9d688658 100644 --- a/rust/cubesql/cubesql/src/compile/mod.rs +++ b/rust/cubesql/cubesql/src/compile/mod.rs @@ -6089,7 +6089,7 @@ limit check_date_minus_date( "2021-03-02 12:34:56.123456789", "2021-02-02 12:00:00.000", - "0 years 0 mons 28 days 0 hours 34 mins 56.123456789 secs", + "0 years 0 mons 28 days 0 hours 34 mins 56.123457 secs", ) .await; @@ -6098,7 +6098,7 @@ limit check_date_minus_date( "2021-02-02 12:00:00.000", "2021-03-02 12:34:56.123456789", - "0 years 0 mons -28 days 0 hours -34 mins -56.-123456789 secs", + "0 years 0 mons -28 days 0 hours -34 mins -56.123457 secs", ) .await; @@ -6106,7 +6106,7 @@ limit check_date_minus_date( "2021-05-02 13:34:56.123456789", "2021-02-02 12:00:00.000", - "0 years 0 mons 89 days 1 hours 34 mins 56.123456789 secs", + "0 years 0 mons 89 days 1 hours 34 mins 56.123457 secs", ) .await; @@ -6114,7 +6114,7 @@ limit check_date_minus_date( "2021-02-02 12:00:00.000", "2021-05-02 13:34:56.123456789", - "0 years 0 mons -89 days -1 hours -34 mins -56.-123456789 secs", + "0 years 0 mons -89 days -1 hours -34 mins -56.123457 secs", ) .await; @@ -6122,7 +6122,7 @@ limit check_date_minus_date( "2023-05-02 13:34:56.123456789", "2021-02-02 12:00:00.000", - "0 years 0 mons 819 days 1 hours 34 mins 56.123456789 secs", + "0 years 0 mons 819 days 1 hours 34 mins 56.123457 secs", ) .await; @@ -6130,7 +6130,7 @@ limit check_date_minus_date( "2021-02-02 12:00:00.000", "2023-05-02 13:34:56.123456789", - "0 years 0 mons -819 days -1 hours -34 mins -56.-123456789 secs", + "0 years 0 mons -819 days -1 hours -34 mins -56.123457 secs", ) .await; @@ -6175,27 +6175,53 @@ limit ) .await; - // TODO: This is the WRONG VALUE. See batch_to_dataframe. // Postgres output: 00:00:00.001 check_date_minus_date( "2000-02-29 00:00:00.000", "2000-02-28 23:59:59.999", - "0 years 0 mons 0 days 0 hours 0 mins 0.1000000 secs", + "0 years 0 mons 0 days 0 hours 0 mins 0.001000 secs", ) .await; - // TODO: This is the WRONG VALUE. check_date_minus_date( "2000-02-25 14:00:00.000", "2000-02-25 13:59:59.999", - "0 years 0 mons 0 days 0 hours 0 mins 0.1000000 secs", + "0 years 0 mons 0 days 0 hours 0 mins 0.001000 secs", ) .await; check_date_minus_date( "2000-02-25 14:00:00.000", "2000-02-25 13:59:59.900", - "0 years 0 mons 0 days 0 hours 0 mins 0.100000000 secs", + "0 years 0 mons 0 days 0 hours 0 mins 0.100000 secs", + ) + .await; + + check_date_minus_date( + "2000-02-25 14:00:00.000123956", + "2000-02-25 13:59:59.900123456", + "0 years 0 mons 0 days 0 hours 0 mins 0.100000 secs", + ) + .await; + + check_date_minus_date( + "2000-02-25 14:00:00.000124956", + "2000-02-25 13:59:59.900123456", + "0 years 0 mons 0 days 0 hours 0 mins 0.100002 secs", + ) + .await; + + check_date_minus_date( + "2000-02-25 13:59:59.900123456", + "2000-02-25 14:00:00.000123956", + "0 years 0 mons 0 days 0 hours 0 mins -0.100000 secs", + ) + .await; + + check_date_minus_date( + "2000-02-25 13:59:59.900123456", + "2000-02-25 14:00:00.000124956", + "0 years 0 mons 0 days 0 hours 0 mins -0.100002 secs", ) .await; } diff --git a/rust/cubesql/cubesql/src/sql/dataframe.rs b/rust/cubesql/cubesql/src/sql/dataframe.rs index e38e9a72febc5..e28bd004b6dc8 100644 --- a/rust/cubesql/cubesql/src/sql/dataframe.rs +++ b/rust/cubesql/cubesql/src/sql/dataframe.rs @@ -560,6 +560,7 @@ pub fn batch_to_dataframe( let milliseconds_part: i32 = (value & 0xFFFFFFFF) as i32; let secs = milliseconds_part / 1000; + let milliseconds_remainder = milliseconds_part % 1000; let mins = secs / 60; let hours = mins / 60; @@ -572,7 +573,7 @@ pub fn batch_to_dataframe( hours, mins, secs, - milliseconds_part % 1000, + milliseconds_remainder * 1000, ))); } } @@ -612,20 +613,37 @@ pub fn batch_to_dataframe( let days: i32 = ((value & 0xFFFFFFFF0000000000000000) >> 64) as i32; let nanoseconds_part: i64 = (value & 0xFFFFFFFFFFFFFFFF) as i64; - let secs = nanoseconds_part / 1000000000; + let secs = nanoseconds_part / 1_000_000_000; + let secs_nano_fraction = (nanoseconds_part % 1_000_000_000) as i32; + let mins = secs / 60; let hours = mins / 60; let secs = secs - (mins * 60); let mins = mins - (hours * 60); + let whole_usecs = secs_nano_fraction / 1000; + let nanos_remainder = secs_nano_fraction % 1000; + + // Postgres supposedly believes in rounding to even. Supposedly because they + // might also mix up fractional seconds with base-2 floating point, affecting + // microsecond rounding. + let usecs: i32; + if secs_nano_fraction < 0 { + usecs = whole_usecs + - (nanos_remainder - (whole_usecs & 1) < -500) as i32; + } else { + usecs = whole_usecs + + (nanos_remainder + (whole_usecs & 1) > 500) as i32; + } + rows[i].push(TableValue::Interval(IntervalValue::new( months, days, hours as i32, mins as i32, secs as i32, - (nanoseconds_part % 1000000000) as i32, + usecs, ))); } } diff --git a/rust/cubesql/pg-srv/src/encoding.rs b/rust/cubesql/pg-srv/src/encoding.rs index 40476ee8ed1e3..1c64d4286037e 100644 --- a/rust/cubesql/pg-srv/src/encoding.rs +++ b/rust/cubesql/pg-srv/src/encoding.rs @@ -221,7 +221,7 @@ impl IntervalValue { )); if self.usecs != 0 { - res.push_str(&format!(".{:06}", self.usecs)) + res.push_str(&format!(".{:06}", self.usecs.abs())) } } @@ -231,18 +231,26 @@ impl IntervalValue { pub fn as_postgresql_str(&self) -> String { let (years, months) = self.extract_years_month(); + // We manually format sign for the case where self.secs == 0, self.usecs < 0. + // We follow assumptions about consistency of hours/mins/secs/usecs signs as in + // as_iso_str here. format!( - "{} years {} mons {} days {} hours {} mins {}.{} secs", + "{} years {} mons {} days {} hours {} mins {}{}.{} secs", years, months, self.days, self.hours, self.mins, - self.secs, + if self.secs < 0 || self.usecs < 0 { + "-" + } else { + "" + }, + self.secs.abs(), if self.usecs == 0 { "00".to_string() } else { - format!("{:06}", self.usecs) + format!("{:06}", self.usecs.abs()) } ) } @@ -346,6 +354,26 @@ mod tests { "0 years 0 mons 0 days 0 hours 0 mins 0.00 secs".to_string() ); + assert_eq!( + IntervalValue::new(0, 0, 0, 0, 1, 23).to_string(), + "0 years 0 mons 0 days 0 hours 0 mins 1.000023 secs".to_string() + ); + + assert_eq!( + IntervalValue::new(0, 0, 0, 0, -1, -23).to_string(), + "0 years 0 mons 0 days 0 hours 0 mins -1.000023 secs".to_string() + ); + + assert_eq!( + IntervalValue::new(0, 0, 0, 0, -1, 0).to_string(), + "0 years 0 mons 0 days 0 hours 0 mins -1.00 secs".to_string() + ); + + assert_eq!( + IntervalValue::new(0, 0, -14, -5, -1, 0).to_string(), + "0 years 0 mons 0 days -14 hours -5 mins -1.00 secs".to_string() + ); + Ok(()) } }