Skip to content

Commit

Permalink
data: Don't add a timezone when coercing to a date
Browse files Browse the repository at this point in the history
If we're coercing a timestamp that has no timezone to a date, make sure
that the resulting date also doesn't have a timezone. This fixes a bug
where we wouldn't be able to do lookups into a date field using
timestamps that had the same date

Refs: ENG-2997
Change-Id: I86bc165fe40c2e19c650df707e852da4c9968526
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/4868
Tested-by: Buildkite CI
Reviewed-by: Fran Noriega <fran@readyset.io>
  • Loading branch information
glittershark committed May 11, 2023
1 parent 3090ad9 commit 33990c7
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
18 changes: 18 additions & 0 deletions logictests/date_params.test
@@ -0,0 +1,18 @@
statement ok
CREATE TABLE `table_1` (`column_2` INT, `column_1` DATE, PRIMARY KEY (`column_2`))


statement ok
INSERT INTO `table_1` (`column_2`, `column_1`) VALUES (0, '2020-01-01')

onlyif readyset
statement ok
create cache from SELECT dayofweek(`table_1`.`column_1`) AS `alias_1` FROM `table_1` WHERE (`table_1`.`column_1` = ?)

graphviz

query rowsort
SELECT count(*) AS `alias_1` FROM `table_1` WHERE (`table_1`.`column_1` = ?)
? = 2020-01-01 00:00:00
----
1
8 changes: 6 additions & 2 deletions readyset-data/src/timestamp.rs
Expand Up @@ -39,7 +39,7 @@ pub const DATE_FORMAT: &str = "%Y-%m-%d";
///
/// Sadly the way chrono implements `DateTime<Tz>` occupies at least 16 bytes, and therefore
/// overflows DfValue. So this type internally stores a [`NaiveDateTime`] with a 3 byte
/// of extra data. Since 3 bytes allow us to store 24 bytes, this is how we use them:
/// of extra data. Since 3 bytes allow us to store 24 bits, this is how we use them:
///
/// 17 bits for the timezone offset (0 to 86_400)
/// 1 bit to signify negative offset
Expand Down Expand Up @@ -350,7 +350,11 @@ impl TimestampTz {
ts.set_subsecond_digits(subsecond_digits as u8);
Ok(DfValue::TimestampTz(ts))
}
DfType::Date => Ok(DfValue::TimestampTz(self.to_chrono().date().into())),
DfType::Date => Ok(if self.has_timezone() {
DfValue::TimestampTz(self.to_chrono().date().into())
} else {
DfValue::TimestampTz(self.to_chrono().date_naive().into())
}),
// TODO(ENG-1833): Use `subsecond_digits` value.
DfType::Time { .. } => Ok(self.to_chrono().naive_local().time().into()),

Expand Down

0 comments on commit 33990c7

Please sign in to comment.