dongjoon-hyun commented on code in PR #59606:
URL: https://github.com/apache/spark/pull/59606#discussion_r4319403270
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/types/ops/TimeTypeParquetOps.scala:
##########
@@ -163,6 +163,15 @@ private[ops] object TimeTypeParquetOps {
override def acceptsValue(value: Any): Boolean =
value.isInstanceOf[LocalTime] &&
isMicrosResolution(value.asInstanceOf[LocalTime])
+ // Below microsecond precision, the reader truncates the stored micros
to the read precision
+ // (see TimeTypeParquetOps.newConverter and TimeVectorUpdater), while a
pushed down predicate
+ // compares the stored micros. E.g. a stored 12:00:00.1005 is read as
12:00:00.100 for
+ // TIME(3), so a pushed down `t = 12:00:00.100` would skip it.
+ override def acceptsReadType(readType: DataType): Boolean = readType
match {
Review Comment:
Thank you. `<` and `>=` are exact only when the bound is on the read grid,
so keeping them needs both the operator and the read precision in the ops. `>`
and `!=` are over-inclusive, but they become under-inclusive under `NOT`:
`ParquetFilters` wraps a negated filter with `FilterApi.not`, and parquet-mr
inverts `gt(v)` to `ltEq(v)`. `BooleanSimplification` usually removes such
`NOT`, but it can be excluded via `spark.sql.optimizer.excludedRules`. So, I'd
like to keep this PR minimal as a correctness fix for `branch-4.3` too, and
recover the pushdown by rewriting the bounds in a new JIRA issue.
--
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]
