Skip to content

Commit

Permalink
fix: include precision information in PrecisionTimestamp and Precisio…
Browse files Browse the repository at this point in the history
…nTimestampTZ literals (#659)

BREAKING CHANGE: changes the message type for Literal PrecisionTimestamp
and PrecisionTimestampTZ

The PrecisionTimestamp and PrecisionTimestampTZ literals were introduced
in #594, and there was some discussion about their contents in
#594 (comment).
In their current form, they only contain a i64 value, which I believe is
meant to be a precision-dependent number of fractional seconds since
epoch. However, the literals don't contain the precision itself, so it's
impossible to interpret a PrecisionTimestamp or PrecisionTimestampTZ
literal without other information. This is in contrast to e.g. varchar,
whose literal does specify the length, or decimal, whose literal
specifies scale and precision. @westonpace curious for your thoughts
since you were part of that original discussion - am I missing something
or is this a bug?

This PR changes the Literal types for PrecisionTimestamp and
PrecisionTimestampTZ to contain a PrecisionTimestamp message instead of
a i64. That message then contains the i64 value as well as the i32 type.
This is a breaking change, but given in their current form these
literals aren't usable (unless I'm missing something), would that be
okay?

Currently I used the same message for both PrecisionTimestamp and
PrecisionTimestampTZ, but I can also make a copy for PTTZ if that'd be
better for forwards-compatibility.
  • Loading branch information
Blizzara authored Jul 11, 2024
1 parent d81a9d0 commit f9e5f9c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
13 changes: 9 additions & 4 deletions proto/substrait/algebra.proto
Original file line number Diff line number Diff line change
Expand Up @@ -814,10 +814,8 @@ message Expression {
VarChar var_char = 22;
bytes fixed_binary = 23;
Decimal decimal = 24;
// If the precision is 6 or less then this is the microseconds since the UNIX epoch
// If the precision is more than 6 then this is the nanoseconds since the UNIX epoch
uint64 precision_timestamp = 34;
uint64 precision_timestamp_tz = 35;
PrecisionTimestamp precision_timestamp = 34;
PrecisionTimestamp precision_timestamp_tz = 35;
Struct struct = 25;
Map map = 26;
// Timestamp in units of microseconds since the UNIX epoch.
Expand Down Expand Up @@ -859,6 +857,13 @@ message Expression {
int32 scale = 3;
}

message PrecisionTimestamp {
// Sub-second precision, 0 means the value given is in seconds, 3 is milliseconds, 6 microseconds, 9 is nanoseconds
int32 precision = 1;
// Time passed since 1970-01-01 00:00:00.000000 in UTC for PrecisionTimestampTZ and unspecified timezone for PrecisionTimestamp
uint64 value = 2;
}

message Map {
message KeyValue {
Literal key = 1;
Expand Down
6 changes: 3 additions & 3 deletions proto/substrait/type.proto
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ message Type {
FixedBinary fixed_binary = 23;
Decimal decimal = 24;
PrecisionTimestamp precision_timestamp = 33;
PrecisionTimestampTZ precision_timestamp_tz = 34;
PrecisionTimestampTZ precision_timestamp_tz = 34; // value is since UNIX epoch in UTC

Struct struct = 25;
List list = 27;
Expand Down Expand Up @@ -164,14 +164,14 @@ message Type {
}

message PrecisionTimestamp {
// Defaults to 6
// Sub-second precision, 0 means the value given is in seconds, 3 is milliseconds, 6 microseconds, 9 is nanoseconds
int32 precision = 1;
uint32 type_variation_reference = 2;
Nullability nullability = 3;
}

message PrecisionTimestampTZ {
// Defaults to 6
// Sub-second precision, 0 means the value given is in seconds, 3 is milliseconds, 6 microseconds, 9 is nanoseconds
int32 precision = 1;
uint32 type_variation_reference = 2;
Nullability nullability = 3;
Expand Down

0 comments on commit f9e5f9c

Please sign in to comment.