uros-b commented on code in PR #56422:
URL: https://github.com/apache/spark/pull/56422#discussion_r3447579489
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFilters.scala:
##########
@@ -252,7 +250,7 @@ class ParquetFilters(
(n: Array[String], v: Any) => FilterApi.eq(
longColumn(n),
Option(v).map(timestampToMillis).orNull)
- case ParquetTimeMicrosType =>
+ case ParquetSchemaType(_: TimeLogicalTypeAnnotation, INT64, 0) =>
Review Comment:
We should probably tighten the ParquetFilters match, as a defensive measure.
`ParquetSchemaType(_: TimeLogicalTypeAnnotation, INT64, 0)` now matches any
INT64 TIME annotation (e.g. TIME(NANOS)), but the value conversion
localTimeToMicros assumes MICROS.
Although it's safe today because the schema converter only ever surfaces
TIME(MICROS) as a readable TimeType (NANOS hits illegalType(), MILLIS is
INT32), and no filter is ever built for those columns. Still, I'd add an
explicit `if t.getUnit == TimeUnit.MICROS` guard so the filter dispatch matches
exactly what the reader supports, rather than relying on that invariant holding
elsewhere.
--
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]
