Blizzara commented on code in PR #11711:
URL: https://github.com/apache/datafusion/pull/11711#discussion_r1723017470
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -1034,29 +1033,61 @@ fn is_time_with_valid_unit(datatype: DataType) -> bool {
)
}
+/// Non-strict Timezone Coercion is useful in scenarios where we can guarantee
+/// a stable relationship between two timestamps of different timezones.
+///
+/// An example of this is binary comparisons (<, >, ==, etc). Arrow stores
timestamps
+/// as relative to UTC epoch, and then adds the timezone as an offset. As a
result, we can always
+/// do a binary comparison between the two times.
Review Comment:
This is only true as long as the timezone is _present_. If tz is None, then
the timestamp is not necessarily relative to UTC epoch (but an "arbitrary"
epoch).
FWIW, I think the changes you've done here make sense (*) - I'm just not
sure if the original behavior of coercing a `(None, Some(tz))` or a `(Some(tz),
None)` are correct. Or well, I'm pretty sure it's not _guaranteed to be
correct_, but maybe it's convenient enough to exist 🤷
(*): though is there need for the "strict" check to exist? I'd assume
coercing two timestamps with timezones would always be fine, given they are
always UTC-epoch based.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
