peterxcli commented on code in PR #5740:
URL: https://github.com/apache/datafusion-comet/pull/5740#discussion_r3943849722
##########
native/core/src/parquet/parquet_support.rs:
##########
@@ -219,22 +231,23 @@ fn parquet_convert_array_impl(
(
Timestamp(TimeUnit::Millisecond, _),
Timestamp(TimeUnit::Microsecond, target_tz),
- ) if top_level && parquet_options.checked_timestamp_overflow => {
+ ) if parquet_options.checked_timestamp_overflow => {
// 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
//
- // The checked conversion is limited to TOP-LEVEL columns. Spark
only avoids the
- // error for filtered-out values through row-group statistics
pruning, and
- // DataFusion's PruningPredicate does not support nested fields
yet, so a checked
- // conversion on a nested field would fail queries whose
predicates Spark prunes
- // (e.g. `WHERE s.ts < X` over an all-overflowing file). Nested
fields keep the
- // pre-existing safe-cast behavior below (overflow -> NULL).
- let micros = array
- .as_primitive::<TimestampMillisecondType>()
- .try_unary::<_, TimestampMicrosecondType, _>(|value|
value.mul_checked(1_000))?
+ // Filtered scans retain safe conversion until DataFusion can
mirror Spark's
+ // pruning paths, including nested predicates (issue #5553).
Review Comment:
```suggestion
// pruning paths, including nested predicates.
```
--
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]
