peterxcli commented on code in PR #5177:
URL: https://github.com/apache/datafusion-comet/pull/5177#discussion_r3882493620
##########
spark/src/test/scala/org/apache/comet/parquet/ParquetReadSuite.scala:
##########
@@ -261,6 +261,56 @@ abstract class ParquetReadSuite extends CometTestBase {
}
}
+ test("TIMESTAMP_MILLIS overflow fails in native scan") {
+ // Spark routes both TimestampType and TimestampNTZType through
LongAsMicrosUpdater:
+ //
https://github.com/apache/spark/blob/v4.2.0/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetVectorUpdaterFactory.java#L140-L164
+ // The updater calls checked millisToMicros for direct and dictionary
values:
+ //
https://github.com/apache/spark/blob/v4.2.0/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetVectorUpdaterFactory.java#L800-L833
+ // Matches Spark's positive and negative overflow cases:
+ //
https://github.com/apache/spark/blob/v4.2.0/sql/core/src/test/resources/sql-tests/inputs/timestamp.sql#L74-L83
+ def isOverflow(error: Throwable): Boolean =
+ Iterator
+ .iterate(error)(_.getCause)
+ .takeWhile(_ != null)
+ .exists(cause =>
Option(cause.getMessage).exists(_.toLowerCase.contains("overflow")))
+
+ Seq(false, true).foreach { dictionaryEnabled =>
+ Seq(92233720368547758L, -92233720368547758L).foreach { millis =>
+ withTempDir { dir =>
+ val path = new Path(dir.toURI.toString, "part-r-0.parquet")
+ val schema = MessageTypeParser.parseMessageType("""
+ |message root {
+ | optional int64 ts(TIMESTAMP_MILLIS);
+ | optional int64 ts_ntz(TIMESTAMP(MILLIS,false));
+ |}
+ |""".stripMargin)
+ val writer = createParquetWriter(schema, path, dictionaryEnabled)
+ val record = new SimpleGroup(schema)
+ record.add(0, millis)
+ record.add(1, millis)
+ writer.write(record)
Review Comment:
Good catch — confirmed: with one row parquet-mr's dictionary writer falls
back to PLAIN because a one-entry dictionary page isn't smaller than the raw
values. Changed the test to write 16 repeated rows and to assert
`getEncodingStats.hasDictionaryEncodedPages == dictionaryEnabled` for every
column chunk of the footer, so the dictionary leg can't silently degrade again.
Verified locally: the suite passes with dictionary pages present when enabled
and PLAIN when disabled, across both overflow signs and ANSI on/off. (c833b72a6)
--
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]
