ajsquared commented on code in PR #7343:
URL: https://github.com/apache/datafusion-comet/pull/7343#discussion_r4428360820
##########
native/core/src/parquet/parquet_exec.rs:
##########
@@ -111,7 +111,20 @@ pub(crate) fn init_datasource_exec(
// `SessionConfig` (an `Arc<ConfigOptions>` plus a small extensions map);
`SessionContext::
// state()` would clone the whole `SessionState`, including the function
registries (~250+
// UDFs) and optimizer rules, on every scan init.
- let session_config = session_ctx.copied_config();
+ let mut session_config = session_ctx.copied_config();
+ if data_filters
+ .as_ref()
+ .is_some_and(|filters| filters.iter().any(contains_make_date))
Review Comment:
[P2] With row-filter pushdown enabled, a date predicate that stays in Spark
can still lose a required overflow. For example, use `WHERE (y = 0 OR
unix_date(make_date(y,1,1)) > 0) AND keep = 1` on a Parquet page containing
both `keep` values, with `y = 6000000` on a `keep = 0` row. Spark evaluates the
date on that row. `unsupportedParentReason` rejects the `OR`, so
`CometNativeScan.buildNativeScanCommon` omits that conjunct but still
serializes `keep = 1`. The check here sees no `MakeDateExpr`, and the reader
drops the row before Spark can raise `ARITHMETIC_OVERFLOW`. Detect `MakeDate`
in the original Catalyst data filters before serialization and carry that
decision into the scan.
##########
native/core/src/parquet/parquet_exec.rs:
##########
@@ -291,11 +304,12 @@ pub(crate) fn init_datasource_exec(
// `propagation.parent_pushdown_result` because Spark's Filter above the
// scan re-evaluates every dataFilter, so No-classified filters stay
// correct without us inserting a FilterExec here. That also makes it safe
- // to drop the filters that call into the JVM first; see `calls_jvm`.
+ // to drop the filters that call into the JVM first; see `calls_jvm`. A
make_date predicate
+ // also stays above the scan so its evaluation and structured errors
remain with the Filter.
let data_filters = data_filters.map(|filters| {
filters
.into_iter()
- .filter(|filter| !calls_jvm(filter))
+ .filter(|filter| !calls_jvm(filter) && !contains_make_date(filter))
Review Comment:
[P2] With `spark.sql.parquet.filterPushdown=false`, a resolved
scalar-subquery predicate can still prune an entire row group and suppress a
required overflow. For example, use `WHERE unix_date(make_date(y,1,1)) >
(SELECT max(v) FROM bounds) AND keep > (SELECT max(v) FROM bounds)`, with a
bound of zero and a row group where every `keep` is zero and one year is
6000000. Spark reads the group and raises on the first conjunct.
`CometNativeScanExec.serializedPartitionData` appends both resolved filters
without checking the pushdown setting. This code removes the date predicate but
still passes `keep > 0` to Parquet for statistics pruning, so Comet can return
an empty result. Apply the same `PARQUET_FILTER_PUSHDOWN_ENABLED` check to
execution-time scalar-subquery filters that `buildNativeScanCommon` applies at
planning time.
--
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]
