ump45nose commented on code in PR #7255:
URL: https://github.com/apache/datafusion-comet/pull/7255#discussion_r4431837872
##########
native/spark-expr/src/conversion_funcs/string.rs:
##########
@@ -1443,18 +1469,33 @@ fn parse_timestamp_to_micros<T: TimeZone>(
Some(days) => days,
None => return Ok(None),
};
- // Compute the timezone offset using epoch as a surrogate probe point.
- // Extreme-year timestamps are only valid with a UTC-like fixed offset
(any DST
- // zone would overflow). Using epoch gives us the standard offset.
- let epoch_probe = NaiveDate::from_ymd_opt(1970, 1, 1)
- .unwrap()
- .and_hms_opt(0, 0, 0)
- .unwrap();
- let tz_offset_secs: i64 = match tz.from_local_datetime(&epoch_probe) {
- LocalResult::Single(dt) => dt.offset().fix().local_minus_utc() as
i64,
- LocalResult::Ambiguous(dt, _) =>
dt.offset().fix().local_minus_utc() as i64,
- LocalResult::None => 0,
+ // Look up the zone offset at the same local civil time on a year
inside chrono's
+ // range that is congruent modulo the 400-year Gregorian cycle (leap
days match).
+ // Far-past and far-future need different eras: a single 1600..=1999
window can
+ // land after a zone's LMT→standard transition (e.g. LA 1883, Tokyo
~1888) for
+ // some far-past residues, and still in the LMT era for far-future
residues
+ // (262543 → 1743), disagreeing with Spark/Java which use the first
offset before
+ // any transition and the permanent/final standard offset for
post-horizon dates.
+ // far-past → 400..=799 (before every IANA transition we care
about → LMT)
+ // far-future → 2200..=2599 (past temporary historical
standard-offset regimes
+ // such as Asia/Colombo +06:00 in 1996–2006; a 2000+rem
probe for
+ // year 262400 landed on 2000 and was 1800s early vs
Spark/Java
+ // +05:30. Summer DST after chrono-tz's last timespan
remains #6816)
+ // Fixed offsets and UTC are unchanged either way.
+ let rem = year.rem_euclid(400);
+ let probe_year = if year > CHRONO_MAX_YEAR {
+ (2200 + rem) as i32
Review Comment:
Fixed in dd77910: the far-future base is now `2400 + rem` (2400..=2799), so
the probe year keeps the input's leap-ness (`262400-02-29` → `2400-02-29`) and
stays past Colombo's +06:00 period.
Tests: the Rust extreme-year test now checks `262400-02-29` in UTC
(`8218382083200000000`) and Asia/Colombo, plus `-262400-02-29`.
`CometDateTimeUtilsSuite` adds both leap days and UTC, and its extreme-year
case now uses `checkSparkAnswerAndOperator`, so a Spark fallback fails it. Rust
spark-expr tests (1313), clippy and fmt pass locally. I can't run the Scala
suite here (no Maven). The previous preflight failure was the npm 404 you
mentioned, not this change.
(AI-assisted reply.)
--
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]
