diff --git a/partiql-logical-planner/src/lower.rs b/partiql-logical-planner/src/lower.rs index 72694d8b..99a16df7 100644 --- a/partiql-logical-planner/src/lower.rs +++ b/partiql-logical-planner/src/lower.rs @@ -849,9 +849,11 @@ impl<'ast> Visitor<'ast> for AstToLogical { Type::TimeTypeWithTimeZone(p) => { Value::DateTime(Box::new(DateTime::from_hh_mm_ss_time_zone(s, p))) } - Type::TimestampType(p) => Value::DateTime(Box::new(DateTime::from_hh_mm_ss(s, p))), + Type::TimestampType(p) => { + Value::DateTime(Box::new(DateTime::from_yyyy_mm_dd_hh_mm_ss(s, p))) + } Type::ZonedTimestampType(p) => { - Value::DateTime(Box::new(DateTime::from_hh_mm_ss_time_zone(s, p))) + Value::DateTime(Box::new(DateTime::from_yyyy_mm_dd_hh_mm_ss_time_zone(s, p))) } _ => todo!("Other types"), }, diff --git a/partiql-value/src/datetime.rs b/partiql-value/src/datetime.rs index 747461dc..369abb9e 100644 --- a/partiql-value/src/datetime.rs +++ b/partiql-value/src/datetime.rs @@ -5,7 +5,7 @@ use std::fmt::{Debug, Formatter}; use std::hash::Hash; use std::num::NonZeroU8; use time::macros::format_description; -use time::{Duration, UtcOffset}; +use time::UtcOffset; #[derive(Hash, PartialEq, Eq, Clone)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -91,39 +91,42 @@ impl DateTime { } pub fn from_yyyy_mm_dd(date: &str) -> Self { + // TODO: for better error mesaging, perhaps could include in our parser or create separate + // parser for these date, time, and timestamp strings let format = format_description!("[year]-[month]-[day]"); let date = time::Date::parse(date, &format).expect("valid date string"); DateTime::Date(date) } pub fn from_hh_mm_ss(time: &str, precision: &Option) -> Self { - let format = format_description!("[hour]:[minute]:[second].[subsecond]"); + let format = format_description!("[hour]:[minute]:[second][optional [.[subsecond]]]"); let time = time::Time::parse(time, &format).expect("valid time string"); DateTime::Time(time, *precision) } pub fn from_hh_mm_ss_time_zone(time: &str, precision: &Option) -> Self { let time_format = format_description!( - "[hour]:[minute]:[second].[subsecond][offset_hour]:[offset_minute]" + "[hour]:[minute]:[second][optional [.[subsecond]]][offset_hour]:[offset_minute]" ); let time_part = time::Time::parse(time, &time_format).expect("valid time with time zone"); let time_format = format_description!( - "[hour]:[minute]:[second].[subsecond][offset_hour]:[offset_minute]" + "[hour]:[minute]:[second][optional [.[subsecond]]][offset_hour]:[offset_minute]" ); let offset_part = time::UtcOffset::parse(time, &time_format).expect("valid time zone"); DateTime::TimeWithTz(time_part, *precision, offset_part) } pub fn from_yyyy_mm_dd_hh_mm_ss(timestamp: &str, precision: &Option) -> Self { - let format = - format_description!("[year]-[month]-[day] [hour]:[minute]:[second].[subsecond]"); + let format = format_description!( + "[year]-[month]-[day] [hour]:[minute]:[second][optional [.[subsecond]]]" + ); let time = time::PrimitiveDateTime::parse(timestamp, &format).expect("valid timestamp string"); DateTime::Timestamp(time, *precision) } pub fn from_yyyy_mm_dd_hh_mm_ss_time_zone(timestamp: &str, precision: &Option) -> Self { - let format = format_description!("[year]-[month]-[day] [hour]:[minute]:[second].[subsecond][offset_hour]:[offset_minute]"); + let format = format_description!("[year]-[month]-[day] [hour]:[minute]:[second][optional [.[subsecond]]][offset_hour]:[offset_minute]"); let time = time::OffsetDateTime::parse(timestamp, &format) .expect("valid timestamp string with time zone"); DateTime::TimestampWithTz(time, *precision) @@ -174,29 +177,30 @@ impl Ord for DateTime { (DateTime::Date(_), _) => Ordering::Less, (_, DateTime::Date(_)) => Ordering::Greater, - (DateTime::Time(l, _lp), DateTime::Time(r, _rp)) => l.cmp(r), - // TODO: sorting using the time precisions + // follow convention of timestamp mentioned in section 12.2 to ignore precision and local utc offset + (DateTime::Time(l, _), DateTime::Time(r, _)) => l.cmp(r), (DateTime::Time(_, _), _) => Ordering::Less, (_, DateTime::Time(_, _)) => Ordering::Greater, - (DateTime::TimeWithTz(l, _lp, lo), DateTime::TimeWithTz(r, _rp, ro)) => { - // TODO: sorting using the time precisions - let lod = Duration::new(lo.whole_seconds() as i64, 0); - let rod = Duration::new(ro.whole_seconds() as i64, 0); - let l_adjusted = *l + lod; - let r_adjusted = *r + rod; - l_adjusted.cmp(&r_adjusted) - } + // follow convention of timestamp mentioned in section 12.2 to ignore precision and local utc offset + (DateTime::TimeWithTz(l, _, _), DateTime::TimeWithTz(r, _, _)) => l.cmp(r), (DateTime::TimeWithTz(_, _, _), _) => Ordering::Less, (_, DateTime::TimeWithTz(_, _, _)) => Ordering::Greater, - // TODO: sorting using the timestamp precisions - (DateTime::Timestamp(l, _lp), DateTime::Timestamp(r, _rp)) => l.cmp(r), + // per section 12.2 of spec, timestamp values are compared irrespective of precision or local UTC offset + (DateTime::Timestamp(l, _), DateTime::Timestamp(r, _)) => l.cmp(r), (DateTime::Timestamp(_, _), _) => Ordering::Less, (_, DateTime::Timestamp(_, _)) => Ordering::Greater, - // TODO: sorting using the timestamp precisions - (DateTime::TimestampWithTz(l, _lp), DateTime::TimestampWithTz(r, _rp)) => l.cmp(r), + // per section 12.2 of spec, timestamp values are compared irrespective of precision or local UTC offset + (DateTime::TimestampWithTz(l, _), DateTime::TimestampWithTz(r, _)) => { + let date_ord = l.date().cmp(&r.date()); + if date_ord != Ordering::Equal { + date_ord + } else { + l.time().cmp(&r.time()) + } + } } } }