dwsmith1983 commented on PR #5365:
URL: 
https://github.com/apache/datafusion-comet/pull/5365#issuecomment-5525514652

   All three addressed in 52efcfe28, with the semantics checked against the 
3.5.9 sources first.
   
   Timestamp cutoff: createTimestampRebaseFuncInRead under EXCEPTION throws 
only below RebaseDateTime.lastSwitchJulianTs, which computes to 
1900-01-01T00:00:00Z from the rebase json (the latest zone switch across all 
602 zones; UTC's own is the 1582 cutover). MILLIS columns convert to micros 
before the check. The native check now uses that cutoff in whichever unit the 
column carries, dates keep lastSwitchJulianDay, and there are tests on both 
sides of the boundary per unit plus the Scala regressions you asked for (a 
metadata-free INT64 at 1969-12-31T23:59:59.999999Z reads verbatim under 
EXCEPTION, one before 1900 fails loudly).
   
   INT64 vs INT96 attribution: you were right that merging the two specs was 
wrong, and the arrow schema alone cannot fix it since the opener runs 
Int96Coercer before the adapter sees anything, so INT96 and INT64 MICROS both 
arrive as Timestamp(us, UTC). The honest source is the footer's 
SchemaDescriptor physical types, so our metadata reader factory now records the 
INT96 leaf ordinals into the file's in-memory key-value metadata when it loads 
the footer, and the adapter attributes each timestamp leaf by depth-first 
ordinal, validated against the leaf count and replacing any pre-existing key 
rather than trusting it. INT64 columns follow the datetime spec and INT96 
columns follow the INT96 spec independently; with attribution available, files 
whose two legacy flags disagree now read correctly instead of refusing. Tests 
cover both directions at the Rust and Spark levels, including an INT96 column 
written through the low-level writer alongside an INT64 one, and the old 
mixed-flags refus
 al test became a four-combination round-trip against Spark's own read.
   
   Nested types: rather than declining, the wrapper now rebuilds struct, list, 
large list, fixed size list, map, and dictionary arrays with each leaf 
transformed under its own policy, preserving nulls and offsets, so STRUCT with 
a DATE leaf under EXCEPTION reads modern and null data natively and only an 
actual ancient leaf value fails. Tests cover modern and null leaves under every 
policy, an ancient leaf rebasing exactly under LEGACY UTC and erroring under 
EXCEPTION, and list of struct of date round trips, at both levels.
   
   On the narrowing: the description now states exactly what still fails at 
execution time instead of reading, which after these changes is a LEGACY file 
with a non UTC or unrecorded writer zone when a timestamp before 1900 actually 
appears, and an EXCEPTION file when an ancient value actually appears. It also 
notes that pruning is lost on every column that receives a policy wrapper, 
modern only LEGACY and check only EXCEPTION files included, and that the 65x 
figure is decoded rows rather than cold bytes or latency.
   
   Battery at this head: 235/235 on both spark-3.5 and spark-4.0, 339 native 
tests, clippy and fmt clean. An independent pass over the change also caught an 
i64 overflow in the nanosecond identity check for the LEGACY UTC path, fixed by 
comparing in days, with a per-unit test.
   


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