adriangb opened a new pull request, #25722: URL: https://github.com/apache/datafusion/pull/25722
# feat: keep optional filters pruning-only in the Parquet post-scan path ## Which issue does this PR close? - Part of #22883. Design notes: https://claude.ai/artifact/SSz7t6hPyhFWp1MDPecVqt - Follow-up of #22384. **Depends on #22384 and #25673.** Review only the top commit. - Fixes the 1.16x TPC-H regression that the benchmark bot found on #22384, when #25681 marks the dynamic filters as optional. ```mermaid graph LR FS["filter_stats commit"] A1["#25673 Optional wrapper"] A2["#25681 producers mark filters"] A3["#25674 gate"] A4["#25682 Parquet consumer"] B1["#25683 FilterExec consumer"] B2["FilterExec reordering (new)"] C2["#25713 split join filter"] P["#22384 post-scan filter"] F["post-scan skips optional filters (new)"] FS --> A3 FS --> B2 A1 --> A2 C2 --> A2 A1 --> A4 A3 --> A4 A1 --> B1 A3 --> B1 A1 --> F P --> F classDef this fill:#f6e7d6,stroke:#b25e12,stroke-width:3px class F this ``` ## Rationale for this change With `pushdown_filters = false`, #22384 makes the scan evaluate all accepted filters for each row after the decode. This includes the hash join dynamic filter. The join checks the same rows again, so this work gives nothing, and it caused a 1.16x TPC-H regression on #22384. Optional filters (#25673) are not needed for correctness. The post-scan filter must not evaluate them. ## What changes are included in this PR? The scan splits the predicate with `split_optional` (#25673): | Case | Required conjunct | Optional conjunct | |---|---|---| | `pushdown_filters = false` | post-scan filter | statistics, page index, bloom filter and file pruning only (as before #22384) | | `pushdown_filters = true`, row filter accepts it | row filter | row filter | | `pushdown_filters = true`, row filter rejects it for a file | post-scan filter | not used for that file | | row filter build error for the whole file | post-scan filter | not used for that file | ```diff // pushdown_filters = false - post_scan = split_conjunction(predicate) + post_scan = split_optional(predicate).0 // required conjuncts only ``` The change is about 10 lines in `opener/mod.rs`, `push_decoder.rs` and `row_filter.rs`. It does not need the gate or `optional_filter_mode` (#25674, #25682). ## What is the testing strategy for this PR? `optional_conjunct_is_never_evaluated_post_scan` (opener unit test) reads a file with each predicate and counts the rows that the post-scan filter sees (`post_scan_rows_pruned + post_scan_rows_matched`): | Predicate | `pushdown_filters` | Output rows | Post-scan rows | |---|---|---|---| | `s IS NOT NULL` (row filter rejects it) | false | 2 | 3 | | `s IS NOT NULL` | true | 2 | 3 | | `id > 1` | false | 2 | 3 | | `Optional(s IS NOT NULL)` | false | 3 | 0 | | `Optional(s IS NOT NULL)` | true | 3 | 0 | | `Optional(id > 1)` | false | 3 | 0 | | `Optional(id > 1)` (row filter accepts it) | true | 2 | 0 | `datasource-parquet` lib tests, `parquet_integration`, `core_integration` and the full sqllogictest suite pass. ## Are there any user-facing changes? No. Without optional filters (before #25681) nothing changes. With them, `pushdown_filters = false` scans do the same row-level work as before #22384 for dynamic filters. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
