Ruchirtripathi commented on code in PR #25099:
URL: https://github.com/apache/datafusion/pull/25099#discussion_r3971536599
##########
datafusion/expr-common/src/casts.rs:
##########
@@ -120,6 +120,21 @@ fn is_lossy_temporal_cast(from_type: &DataType, to_type:
&DataType) -> bool {
if is_date_type(from_type) && is_date_type(to_type) {
return false;
}
+ if let (DataType::Timestamp(_, from_tz), DataType::Timestamp(_, to_tz)) =
Review Comment:
Thanks for the thorough review! I’ve updated the PR to address all four
points:
1. Updated the `is_lossy_temporal_cast` doc comment to clearly explain the
new timezone rules.
2. Clarified that timezone shifts are mathematically reversible (bijective),
so they are not actually lossy.
3. Added comments explaining that returning `true` here is an intentionally
conservative guard. Shifting the literal would keep pushdown and pruning
working, but blocking the unwrap is a safer immediate fix to guarantee
correctness.
4. Refactored the logic to use a clean `match (from_tz, to_tz)` statement
and removed the `unwrap()`.
Please take a look at the latest commits!
--
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]
