[PR] fix: [branch-1.0] seed native Parquet scan reader options from session config (#5107) [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove opened a new pull request, #5153:
URL: https://github.com/apache/datafusion-comet/pull/5153

   ## Which issue does this PR close?
   
   Backport of #5107 (merged to `main` as `0b2a50608`) onto `branch-1.0`. Closes #4990 for the 1.0.0 release branch.
   
   ## 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`.
   
   That means `spark.comet.parquet.rowFilterPushdown.enabled` — a user-facing config on `branch-1.0` — does not actually enable row-level pushdown at the native scan, and the `spark.comet.datafusion.execution.parquet.*` escape hatch (behind `spark.comet.exec.respectDataFusionConfigs`) is inert for every reader option. Worth carrying into 1.0.0 rather than shipping configs that silently do nothing.
   
   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, even when a developer set those keys explicitly for testing.
   
   ## What changes are included in this PR?
   
   A clean `git cherry-pick -x` of `0b2a50608`. No conflicts and no adaptation was needed — `git diff` against the commit on `main` is byte-identical.
   
   - `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 per-file execution path (`pushdown_filters`, `reorder_filters`, `force_filter_selections`, `bloom_filter_on_read`, `max_predicate_cache_size`, `enable_page_index`, `pruning`). Deliberately an allowlist rather than a wholesale `global = session.clone()`, so a new DataFusion reader option is ignored until someone checks it is safe for Spark semantics. `coerce_int96`/`coerce_int96_tz` stay hardcoded afterward, non-overridable, for Spark INT96 compatibility.
   - Not wired up: `schema_force_view_types`, `binary_as_string`, `skip_metadata`, `global.metadata_size_hint`. All four are only consumed by `ParquetFormat::infer_schema`, which Comet's scan construction never calls (Comet always supplies its own schema translated from Spark's catalog), so seeding them would be a no-op.
   - `init_datasource_exec` now computes `session_ctx.copied_config()` once and reuses it for both `get_options()` and `try_pushdown_filters`, instead of calling `session_ctx.state()` — which clones the whole `SessionState`, including the function registries and optimizer rules — on every scan init.
   - `native/core/src/execution/jni_api.rs`: the `rowFilterPushdown.enabled` translation moves to *before* the `spark.comet.datafusion.*` pass-through loop, so an explicit override of `pushdown_filters`/`reorder_filters` wins instead of being silently forced back to `true`.
   
   ## How are these changes tested?
   
   By the tests added in #5107, verified on this branch:
   
   - `cargo test -p datafusion-comet --lib parquet_exec` — 2 passed, including the new `seeds_allowlisted_fields_from_session_but_protects_spark_compat_fields`, which asserts `get_options()` carries the allowlisted fields through from the session while keeping `coerce_int96`/`coerce_int96_tz` hardcoded.
   - `CometNativeReaderSuite` under `-Pspark-3.5` — 54 passed, 2 canceled (pre-existing version-gated cancellations). Includes the three new tests: row-level pushdown only fires at the native scan once `rowFilterPushdown.enabled` is set (the core #4990 regression), an explicit `pushdown_filters=false` override wins over that flag, and an explicit `pruning=false` override actually disables row-group statistics pruning.
   - `cargo clippy --all-targets --workspace -- -D warnings` — clean.
   
   JDK 11 locally, hence `-Pspark-3.5`; the Spark 4.x lanes are left to CI.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix: [branch-1.0] seed native Parquet scan reader options from session config (#5107) [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove merged PR #5153:
URL: https://github.com/apache/datafusion-comet/pull/5153


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org