Ruchirtripathi commented on code in PR #25099:
URL: https://github.com/apache/datafusion/pull/25099#discussion_r4103401922
##########
datafusion/expr-common/src/casts.rs:
##########
@@ -113,17 +113,54 @@ fn is_date_type(data_type: &DataType) -> bool {
/// `Date64` carrying sub-day milliseconds would lose them. This is not a
licence to
/// drop them - [`try_cast_numeric_literal`] returns `None` for a `Date64`
value not
/// divisible by 86_400_000, so an inexact `Date64` -> `Date32` fold never
happens.
+///
+/// **Timezone Shifts:**
+/// Conversions between timezone-naive and timezone-aware timestamps are
+/// mathematically bijective (shifting the physical value by the timezone
offset),
+/// rather than many-to-one lossy. However, we return `true` here to block
unwrapping
+/// as an intentionally conservative guard. If we returned `false`,
`unwrap_cast_in_comparison`
+/// would strip the cast but fail to shift the underlying literal, returning
incorrect
+/// query results. (A robust alternative would be to allow the unwrap and
shift the literal,
+/// preserving pushdown and pruning.) Only UTC-equivalent timezones (where the
shift is
+/// exactly zero) are allowed to bypass this guard.
Review Comment:
Thanks for the detailed explanation! You're totally right about the DST gaps
and folds breaking the bijection.
I've updated the doc comment with your exact wording to make the reasoning
crystal clear.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
