sunchao commented on code in PR #5177:
URL: https://github.com/apache/datafusion-comet/pull/5177#discussion_r3930958404
##########
native/core/src/parquet/parquet_exec.rs:
##########
@@ -94,6 +94,14 @@ pub(crate) fn init_datasource_exec(
);
spark_parquet_options.use_field_id = use_field_id;
spark_parquet_options.ignore_missing_field_id = ignore_missing_field_id;
+ // Spark can discard filtered-out values before timestamp conversion using
statistics,
+ // dictionary, and row-level filters. Comet cannot mirror every pruning
path, so applying
+ // checked conversion in a filtered scan can fail on values Spark never
reads. Preserve the
+ // existing safe cast for filtered scans and use checked conversion only
when every value is
+ // necessarily read.
+ spark_parquet_options.checked_timestamp_overflow = !data_filters
Review Comment:
**[P2] Preserve the filtered-scan fallback when native serialization drops a
Spark filter**
`checked_timestamp_overflow` is based on the serialized `data_filters`, but
that list can be empty even when Spark has a Parquet-pushable filter. For
example, with `spark.comet.expression.IsNull.enabled=false`, consider `SELECT
ts ... WHERE id IS NULL` over a row group with optional `id` values that are
all non-null and an overflowing `TIMESTAMP_MILLIS` `ts`. Spark pushes
`IsNull(id)` into Parquet and prunes the group using its zero null count.
`CometNativeScan` omits the disabled expression from its native payload without
rejecting the scan, so this flag becomes true and the checked conversion errors
before the remaining Spark filter can return zero rows. The base safe cast
would return zero rows.
Could we retain whether Spark originally supplied data filters even if none
serialize, and add a regression for that case? I verified the source paths but
did not run this exact configuration end to end.
--
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]
