mbutrovich opened a new pull request, #5107:
URL: https://github.com/apache/datafusion-comet/pull/5107
## Which issue does this PR close?
Closes #4990.
## Rationale for this change
Comet's native Parquet scan built a fresh `TableParquetOptions::new()` per
scan and never looked at the session's `datafusion.execution.parquet.*` config.
`pushdown_filters` appeared to work only because
`ParquetSource::try_pushdown_filters` happens to OR the session value in
independently; every other reader option (`reorder_filters`,
`force_filter_selections`, `bloom_filter_on_read`, `max_predicate_cache_size`,
`enable_page_index`, `pruning`) was silently inert, including when driven by
`spark.comet.parquet.rowFilterPushdown.enabled`.
Fixing this also exposed a precedence bug: the `rowFilterPushdown.enabled`
flag unconditionally overwrote the raw
`spark.comet.datafusion.execution.parquet.{pushdown_filters,reorder_filters}`
escape hatch (gated by `spark.comet.exec.respectDataFusionConfigs`), even when
a developer set those keys explicitly for testing.
## What changes are included in this PR?
- `native/core/src/parquet/parquet_exec.rs`: `get_options()` now takes the
session's `ParquetOptions` and seeds `table_parquet_options.global` for the
fields confirmed to reach `ParquetSource`'s actual execution path
(`pushdown_filters`, `reorder_filters`, `force_filter_selections`,
`bloom_filter_on_read`, `max_predicate_cache_size`, `enable_page_index`,
`pruning`). `coerce_int96`/`coerce_int96_tz` stay hardcoded afterward,
non-overridable, for Spark INT96 compatibility. `session_ctx.state()` is now
computed once in `init_datasource_exec` and reused, instead of being cloned
twice.
- Deliberately not wired up: `schema_force_view_types`, `binary_as_string`,
`skip_metadata`, and `global.metadata_size_hint`. All four are only consumed by
`ParquetFormat::infer_schema`, which Comet's scan construction never calls, so
seeding them would be a no-op.
- `native/core/src/execution/jni_api.rs`: `rowFilterPushdown.enabled` now
only forces `pushdown_filters`/`reorder_filters` to `true` when the
corresponding `spark.comet.datafusion.execution.parquet.*` key wasn't already
set explicitly, so the escape hatch's explicit override wins.
- `spark/src/main/scala/org/apache/comet/CometConf.scala`: documented that
`rowFilterPushdown.enabled`'s derived settings can be overridden via the escape
hatch.
## How are these changes tested?
- New Rust unit test in `parquet_exec.rs` asserting `get_options()` carries
the allowlisted fields through from the session while keeping the Spark-compat
fields hardcoded.
- Two new Scala tests in `CometNativeReaderSuite`: one confirming row-level
pushdown only fires at the native scan once `rowFilterPushdown.enabled` is set
(the core #4990 regression), and one confirming an explicit
`spark.comet.datafusion.execution.parquet.pushdown_filters=false` override wins
over the flag.
--
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]
