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

   This is a light fully automated review since there are so many PRs open.
   
   `leaf_policies` in `native/core/src/parquet/datetime_rebase.rs:689` picks 
the timestamp policy from the physical Arrow type and treats every 
timezone-free physical timestamp as `Corrected`, on the premise in the doc 
comment at line 687 that Spark never rebases these. Spark keys on the requested 
type instead. In `ParquetVectorUpdaterFactory` on 3.5.8, 4.0.1 and 4.1.1, an 
INT64 column requested as `TimestampType` gets `LongWithRebaseUpdater` (or 
`LongAsMicrosRebaseUpdater` for millis) whenever the file's mode is not 
CORRECTED, and `isTimestampTypeMatched` checks only the unit, never 
`isAdjustedToUTC`. Comet's adapter accepts that pairing by reinterpreting the 
micros (`parquet_support.rs:304`), so the column is read with no rebase wrapper 
at all. Take a Delta table whose schema has `ts TIMESTAMP` while its files 
store `ts` as INT64 `TIMESTAMP(MICROS, isAdjustedToUTC=false)` with no Spark 
footer metadata, for example pyarrow-written Parquet converted with `CONVERT TO 
DELTA` under `spa
 rk.sql.parquet.inferTimestampNTZ.enabled=false`. On Spark 3.5 with the default 
`EXCEPTION` read mode, a `1800-01-01 00:00:00` value makes Spark fail with the 
ancient-datetime `SparkUpgradeException`, while the native Delta scan returns 
the row. Under `LEGACY`, Spark rebases pre-1582 values and the native scan 
returns them unchanged. This is the mirror image of the INT96 `TIMESTAMP_NTZ` 
case from the latest review. Could the physical-to-requested leaf pairing 
choose the timestamp policy from the requested type in both directions? A 
metadata-free `TIMESTAMP(MICROS, false)` file read as `TIMESTAMP` under 
`EXCEPTION` would make a good regression next to that one.
   


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