Ruchirtripathi commented on code in PR #25099:
URL: https://github.com/apache/datafusion/pull/25099#discussion_r3971401718


##########
datafusion/expr-common/src/casts.rs:
##########
@@ -120,10 +120,34 @@ 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)) =
+        (from_type, to_type)
+        && from_tz.is_some() != to_tz.is_some()
+    {
+        let tz = from_tz.as_ref().or(to_tz.as_ref()).unwrap().as_ref();
+        if !is_zero_offset_timezone(tz) {
+            return true;
+        }
+    }
     (is_date_type(from_type) && to_type.is_temporal())
         || (is_date_type(to_type) && from_type.is_temporal())
 }
 
+/// Returns true if the timezone is known to have a fixed zero offset from UTC.
+///
+/// This is used to determine if a cast between a timezone-aware and 
timezone-naive
+/// timestamp is lossy. If the timezone is strictly UTC-equivalent, the cast is
+/// a lossless re-labeling of the integer value.
+fn is_zero_offset_timezone(tz: &str) -> bool {
+    match tz {
+        // Standard UTC identifiers
+        "UTC" | "Etc/UTC" | "GMT" | "Etc/GMT" | "Greenwich" | "Z" => true,
+        // Common fixed offset zero strings parsed by Arrow
+        "+00:00" | "-00:00" | "+0:00" | "-0:00" => true,
+        _ => false,

Review Comment:
    The arrow::array::timezone::Tz enum only allows fetching the offset 
dynamically for a specific NaiveDateTime via offset_from_utc_datetime(). 
However, we cannot simply instantiate an arbitrary date (like the Unix Epoch) 
and check if the offset is zero because geographic timezones like Europe/London 
evaluate to an offset of 0 during winter time. This would create false 
positives during query planning.
   
   Because we need to evaluate this statically during optimization (where we 
don't want to instantiate values just to check offsets), explicitly 
whitelisting the known permanent zero-offset strings is currently the safest 
and most robust approach.



-- 
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]

Reply via email to