zhuqi-lucas opened a new pull request, #23701:
URL: https://github.com/apache/datafusion/pull/23701

   ## Which issue does this PR close?
   
   Not a replacement for #22384 — this PR is a **companion**: rebase of #22384 
on latest `main` + one additional commit applying @adriangb's own tactical 
suggestion to unblock its benchmark regressions.
   
   ## What's in here
   
   Two commits, both preserved with the original author:
   
   1. **`feat(parquet-datasource): always accept pushable filters, run rejected 
conjuncts post-scan`** — @adriangb's #22384 commit, rebased onto current `main` 
(5 `.slt` conflict files resolved by taking the PR's version).
   
   2. **`perf(hash-join): split membership from bounds in dynamic filter 
pushdown`** — new. Adds a config gate that lets `HashJoinExec` publish only the 
bounds portion of its dynamic filter by default, keeping the expensive 
`InListExpr` / `HashTableLookupExpr` membership branch behind an opt-in knob.
   
   ## Rationale
   
   @adriangb noted in [#23420 
comment](https://github.com/apache/datafusion/pull/23420#issuecomment-4958698430):
   
   > if the join is a 90% match you end up just paying the hashing price twice 
for no benefit … one path forward isn't to attack that specific case by e.g. 
splitting out the `min/max` range dynamic filters that `HashJoinExec` pushes 
down from the hash table ones and then we could turn off the hash table ones by 
default.
   
   That's exactly what commit 2 does. Mechanical picture:
   
   | Portion | Cost per row | Value |
   |---|---|---|
   | Bounds (`col >= min AND col <= max`) | ~2 ns | Drives RG-level statistics 
/ bloom pruning |
   | Membership (`InListExpr` / hash-table) | ~50–100 ns | Row-level pruning; 
duplicates HashJoin's own hash lookup on match-heavy joins |
   
   #22384's contract change makes the combined `bounds AND membership` filter 
always evaluate (via `RowFilter` when `pushdown_filters=true`, via 
`PostScanFilter` otherwise), so the membership cost that previously never ran 
on the `pushdown_filters=false` path now shows up on every scanned batch.
   
   ## What the fix does
   
   New config knob: 
`datafusion.optimizer.enable_hash_join_dynamic_membership_filter` (default 
`false`). When off, `SharedBuildAccumulator::build_filter` skips 
`create_membership_predicate` in both CollectLeft and Partitioned finalize 
paths and publishes only the bounds expression. RG-level pruning unaffected.
   
   ## Tests
   
   Two new unit tests in `shared_bounds.rs`:
   - `collect_left_with_gate_off_publishes_bounds_only`
   - `collect_left_with_gate_on_publishes_bounds_and_membership`
   
   All existing coverage still passes: 398 hash_join tests, 1559 physical-plan 
lib tests, 160 parquet lib tests, 213 parquet_integration tests, 
information_schema.slt.
   
   @adriangb's two regression tests from #22384 still green:
   - `build_row_filter_surfaces_rejected_struct_conjunct`
   - `rejected_struct_conjunct_runs_post_scan_not_dropped`
   
   ## Benchmark plan
   
   Marked Draft because the point of this PR is to demonstrate that #22384's 
benchmark regression is closable by this tactical split — I'll trigger `run 
benchmarks` next and post the numbers.
   
   ## Credit
   
   Foundation is entirely @adriangb's work in #22384 — this PR preserves that 
commit as-is (author = Adrian Garcia Badaracco, rebased on main). The tactical 
follow-up is on top.
   
   Related: #22384 (foundation), #22237 (full adaptive), #23532 (merged Layer 1 
fix), #23420 (discussion).


-- 
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]

Reply via email to