spark-test commented on code in PR #59699:
URL: https://github.com/apache/spark/pull/59699#discussion_r4348007047
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcFilterSuite.scala:
##########
@@ -333,6 +338,69 @@ class OrcFilterSuite extends OrcTest with
SharedSparkSession {
}
}
+ test("SPARK-60449: filter pushdown - timestamp with sub-microsecond digits")
{
+ // Another engine (e.g. Hive) can write sub-microsecond digits to an ORC
timestamp column, and
+ // Spark reads the column as TimestampType by truncating every value to
microseconds. So the
+ // pushed down predicates must not skip the stripes whose truncated values
satisfy them.
+ withTempPath { dir =>
+ val schema = TypeDescription.fromString("struct<ts:timestamp>")
+ Seq(
+ "part-0.orc" -> Seq("2020-05-25 10:11:12.123456789", null),
Review Comment:
**Nit (P3):** Both non-null fixture values are 789 ns past a microsecond, so
neither edge of the `[v, v + 999ns]` range in `OrcFilters.timestampRange` is
exercised. If `ts.getNanos + 999` is changed to `+ 800`, every count and
`stripSparkFilter` assertion here still passes, yet a stripe storing
`2020-05-25 10:11:12.123456999` (read as `.123456`) would be skipped for `ts =
TIMESTAMP'2020-05-25 10:11:12.123456'`, because ORC sees `upper < min`. If it
is changed to `+ 1000`, `ts > a` still returns 1 here, yet a stripe whose
maximum is exactly `.123457000` would be skipped, since `NOT (ts <= a +
1000ns)` evaluates to NO. Could you add a value at the inclusive upper edge
(e.g. `.123456999`) and one at the next microsecond (e.g. `.123457000`, as the
maximum of its own file), with matching `=`, `<=`, `>` and `IN` expectations,
so an off-by-one in the offset fails the test?
**Verification:**
- **Regression:** With native ORC filter pushdown on (vectorized and
non-vectorized, Java 8 datetime API on and off), queries against ORC files
storing values at v + 999ns and at v + 1us return the counts implied by
microsecond truncation for =, <=, >, and IN, and the test fails if the range
upper bound is changed from v + 999ns.
--
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]
