ump45nose commented on code in PR #7089:
URL: https://github.com/apache/datafusion-comet/pull/7089#discussion_r4326157320


##########
native/core/src/parquet/schema_adapter.rs:
##########
@@ -687,6 +690,21 @@ fn spark_has_updater(
         return false;
     }
 
+    // Spark has no updater that reads TIMESTAMP(NANOS) as a timestamp; it 
reads it only as a
+    // long, under `spark.sql.legacy.parquet.nanosAsLong`. INT96 is coerced to 
microseconds, so
+    // a nanosecond timestamp is TIMESTAMP(NANOS), except below a list, where 
an INT96 leaf of a
+    // legacy list layout is left as a timezone-free nanosecond timestamp 
(#7029). There only
+    // the UTC-adjusted form is known to be TIMESTAMP(NANOS). See #7034.
+    if let (
+        DataType::Timestamp(TimeUnit::Nanosecond, physical_tz),

Review Comment:
   Fixed in 41b8594: the rule now only rejects leaves the footer check tags as 
`TIMESTAMP(NANOS)` from the schema descriptor, so a raw INT96 leaf passes. 
Covered by `int96_reads_as_timestamp_with_row_filters` (your fixture shape, `id 
> 0`, row filters on and off) and the JVM test `native scan reads INT96 
timestamps with row filter pushdown like Spark`. (AI-assisted reply.)
   



##########
native/core/src/parquet/schema_adapter.rs:
##########
@@ -687,6 +690,21 @@ fn spark_has_updater(
         return false;
     }
 
+    // Spark has no updater that reads TIMESTAMP(NANOS) as a timestamp; it 
reads it only as a
+    // long, under `spark.sql.legacy.parquet.nanosAsLong`. INT96 is coerced to 
microseconds, so
+    // a nanosecond timestamp is TIMESTAMP(NANOS), except below a list, where 
an INT96 leaf of a
+    // legacy list layout is left as a timezone-free nanosecond timestamp 
(#7029). There only
+    // the UTC-adjusted form is known to be TIMESTAMP(NANOS). See #7034.
+    if let (
+        DataType::Timestamp(TimeUnit::Nanosecond, physical_tz),
+        DataType::Timestamp(TimeUnit::Microsecond, _),
+    ) = (physical_type, target_type)
+    {
+        if physical_tz.is_some() || !in_list {

Review Comment:
   Fixed in 41b8594 the same way: a timezone-bearing INT96 leaf isn't tagged, 
so it's accepted. `int96_with_arrow_timezone_hint_reads_as_timestamp` writes 
INT96 with an `ARROW:schema` hint of `timestamp('ns', tz='UTC')` and reads it 
back with and without row filters. (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]

Reply via email to