sunchao commented on code in PR #7219:
URL: https://github.com/apache/datafusion-comet/pull/7219#discussion_r4433043674


##########
native/spark-expr/src/utils.rs:
##########
@@ -206,6 +206,78 @@ pub(crate) fn resolve_local_datetime(tz: &Tz, 
local_datetime: NaiveDateTime) ->
     }
 }
 
+/// Microseconds in one 400-year Gregorian cycle (146097 days). Calendars, 
weekdays and
+/// therefore recurring time-zone rules repeat with this period.
+const MICROS_PER_GREGORIAN_CYCLE: i64 = 146_097 * 86_400 * 1_000_000;
+const MILLIS_PER_GREGORIAN_CYCLE: i64 = 146_097 * 86_400 * 1_000;
+
+/// How many Gregorian cycles from the epoch a value outside chrono's year 
range
+/// (-262144..=262143) is moved to before its offset is looked up. 600 cycles 
is 240,000 years,
+/// inside chrono's range on both sides of 1970.
+const IN_RANGE_CYCLES: u64 = 600;
+
+/// Year used for offset lookups when the wall time is outside chrono's year 
range.
+///
+/// `chrono-tz` stops applying DST transitions after about 2099, while 
Spark/`java.time`
+/// keep applying the last zone rules forever. Looking the offset up at year 
2000 (inside
+/// that window, and a leap year so 29 February stays valid) matches Spark for 
far-future
+/// and far-past seasonal zones. Looking it up at the cycle-shifted year 
(~±240000) would
+/// silently use standard time only and disagree with Spark by an hour on 
summer dates.
+const TZ_RULE_LOOKUP_YEAR: i32 = 2000;
+
+/// Same local month/day/time as `local`, but in [`TZ_RULE_LOOKUP_YEAR`].
+fn project_to_tz_rule_year(local: NaiveDateTime) -> NaiveDateTime {
+    let date = local.date();
+    // 2000 is a leap year, so every month/day chrono can represent (including 
29 Feb) maps.
+    let projected = date
+        .with_year(TZ_RULE_LOOKUP_YEAR)
+        .expect("TZ_RULE_LOOKUP_YEAR accepts every chrono month/day");
+    projected.and_time(local.time())
+}
+
+/// Converts a local (timestamp without time zone) value to UTC.
+///
+/// Spark (`java.time`) handles the whole `i64` range, while chrono only 
represents years
+/// -262144 to 262143. For a value outside chrono's range, the wall time is 
moved towards
+/// the epoch by whole 400-year cycles until chrono can represent it, then the 
offset is
+/// looked up at the same month/day/time in [`TZ_RULE_LOOKUP_YEAR`] (see that 
constant)
+/// and applied to the original value. The number of cycles comes from the 
value, so this
+/// covers the whole `i64` range for both microseconds and milliseconds.
+/// Overflowing `i64` while applying the offset raises `long overflow`, as 
Spark does.
+fn local_to_utc(
+    value: i64,
+    unit_per_cycle: i64,
+    to_local: impl Fn(i64) -> Option<NaiveDateTime>,
+    to_utc: impl Fn(NaiveDateTime) -> i64,
+    wall_as_epoch: impl Fn(NaiveDateTime) -> i64,
+) -> Result<i64, ArrowError> {
+    if let Some(local_datetime) = to_local(value) {
+        return Ok(to_utc(local_datetime));
+    }
+    let shifted = shift_towards_epoch(value, unit_per_cycle);
+    let local_datetime = to_local(shifted).ok_or_else(|| 
datetime_cast_err(value))?;
+    let lookup = project_to_tz_rule_year(local_datetime);

Review Comment:
   With `America/Los_Angeles`, Java gives noon on `+280000-03-20` an offset of 
`-07:00`. Looking up March 20 in 2000 gives `-08:00` because US daylight saving 
began in April that year. Java also gives `-280000-07-20` its pre-transition 
offset of `-07:52:58`, while July 2000 gives `-07:00`. Could out-of-range 
conversions use Spark-compatible first/recurring zone rules or fall back for 
cases the native implementation cannot resolve?



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