peterxcli commented on code in PR #5177:
URL: https://github.com/apache/datafusion-comet/pull/5177#discussion_r3886100230
##########
native/core/src/parquet/parquet_support.rs:
##########
@@ -195,6 +195,21 @@ fn parquet_convert_array(
list_arr.nulls().cloned(),
)))
}
+ (
+ Timestamp(TimeUnit::Millisecond, _),
+ Timestamp(TimeUnit::Microsecond, target_tz),
+ ) => {
+ // Spark's Parquet reader calls the checked `millisToMicros`
conversion for both
+ // direct and dictionary values, independent of CAST evaluation
mode:
+ //
https://github.com/apache/spark/blob/v4.2.0/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetVectorUpdaterFactory.java#L817-L833
+ // `millisToMicros` uses `Math.multiplyExact`:
+ //
https://github.com/apache/spark/blob/v4.2.0/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala#L103-L108
+ let micros = array
+ .as_primitive::<TimestampMillisecondType>()
+ .try_unary::<_, TimestampMicrosecondType, _>(|value|
value.mul_checked(1_000))?
+ .with_timezone_opt(target_tz.clone());
Review Comment:
Confirmed and fixed — thanks for the thorough repro. The predicate's column
was wrapped in `CometCastColumnExpr`, which is opaque to DataFusion's pruning
analyzer, so the row group Spark prunes from millisecond statistics was being
read and converted. The fix rewrites predicate comparisons over a
`TIMESTAMP_MILLIS` file column into the millisecond domain (exact integer
rescaling of the literal, like Spark's `ParquetFilters` pushing predicates in
the file's physical unit), plus `IS NULL`/`IS NOT NULL` unwrapping. Pruning
works again, predicate evaluation never converts file values, and the scan
output conversion stays checked, so values actually read still fail like
`millisToMicros`.
Added your filtered case as a regression test over dictionary × ANSI ×
`rowFilterPushdown` (8 configs), and native unit tests pinning the rounding
table for all six comparison operators, both operand orders, and
negative/sub-millisecond literals.
One deliberate divergence to note: with row-filter pushdown on and a
non-pruned row group, rows the filter discards are no longer converted, so
Comet can succeed where Spark (which converts the whole row group during
decode) throws — the benign direction of "errors only for values actually
read." (d46519298)
--
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]