feat: prune-only transfer of parent and dynamic filters across HashJoinExec keys (left, right and mark joins) - #25550
jayzhan211 wants to merge 1 commit into
Conversation
…for left, right and mark joins
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25550 +/- ##
==========================================
+ Coverage 82.39% 82.41% +0.02%
==========================================
Files 1138 1138
Lines 434592 435317 +725
Branches 434592 435317 +725
==========================================
+ Hits 358073 358761 +688
+ Misses 54853 54844 -9
- Partials 21666 21712 +46 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
kosiew
left a comment
There was a problem hiding this comment.
Thanks for working on this. The prune-only distinction looks like a useful direction, and the added differential, NULL, join-filter, dynamic-filter, and fetch coverage is helpful.
I found one correctness issue with outer joins when the join has a fetch. Pruning the non-preserved side can change which rows reach the fetch, which can then change the rows seen by the parent filter. I think we need to disable this transfer for Left and Right joins when fetch is set. Mark joins do not have the same multiplicity issue, so they can still support prune-only transfer with fetch.
I left the details and a concrete example inline.
| // side can turn a row that fails the filter from matched into | ||
| // unmatched (NULL-extended, or mark = false), but every row | ||
| // derived from it still fails the filter, which stays above the | ||
| // join. It never adds output rows, so a `fetch` on this join or |
There was a problem hiding this comment.
I think we need to guard PruneOnly for outer joins when this join has fetch. Pruning matching rows from the non-preserved side can remove rows that would otherwise fill the fetch, which can expose a later row that passes the parent filter and change the query result.
For example, consider left rows (bb, aa), right rows (bb, bb, aa), a parent filter lk = 'aa', and fetch = 2. Without transferring rk = 'aa', the two bb matches can fill the fetch, so the parent filter returns no rows. With the transferred filter, those right-side bb rows are pruned. The aa match can then reach the fetch, so the parent filter can return it.
Could we avoid prune-only transfer for Left and Right when self.fetch.is_some()? Mark joins should still be safe because they do not have this match-multiplicity issue. It would also be good to update the fetch test to compare supported and unsupported plans using the same fetch, with a failing-side key ordered first so this case is exercised directly.
| async fn test_hashjoin_prune_only_transfer_with_fetch() { | ||
| use datafusion_common::JoinSide; | ||
|
|
||
| for join_type in [JoinType::Left, JoinType::Right] { |
There was a problem hiding this comment.
Once the outer-join fetch case is guarded, could we also add LeftMark and RightMark to a same-fetch differential test? Mark joins keep one output row per preserved-side input row, so this would document why prune-only transfer is still safe for mark joins when fetch is present.
Which issue does this PR close?
Follow-up to #25255, which left outer joins for later.
Rationale for this change
#25255 lets
HashJoinExecrewrite a parent filter over one side's join keys onto the other side's keys and push it to both inputs, for inner and semi joins. There every output row has equal keys on both sides, so the copy is as good as the original.Left, right and mark joins also emit unmatched rows of their preserved side, so the copy is not as good as the original. It is still useful as a prune-only filter on the other side's input:
Why this is safe. A right row that fails
rk = 'aa'can only match left rows that faillk = 'aa': their keys are equal, also underNullEqualsNullwhere both values are the same NULL, and a join filter only removes pairs. So every left row that passes the filter keeps exactly the same matches. A left row that fails it may turn from matched into NULL-extended (or mark = false), but every row derived from it still fails the filter, and the filter stays above the join unless the preserved side's scan accepted it. The copy never adds output rows, so afetchon the join or above it cannot lose rows that pass.Rules:
LEFT JOIN ... WHERE r.k IS NULLmust not becomel.k IS NULL, because the non-preserved key is also NULL for unmatched rows;Full, not for null-aware joins (a NULL probe key decides the whole NOT IN result, and the copy is not true for NULL);fetchbetween the join and the filter can be filled by them and displace rows that pass. Example:l.k = {0, 0, 10, 11},r.k = {10, 11}, a TopK filterk < 2from another union branch,fetch = 2on the join. Anti joins need fetch guards on the operators that forward filters first.The main beneficiary is a dynamic filter from a join above that lands on the preserved key of a left or mark join below: it now prunes the other input too.
Benchmarks
TL;DR: neutral on TPC-DS SF1, TPC-H SF10 and JOB in both parquet modes; no regression. The change applies to few plans in these suites, and where it applies (TPC-DS Q80) it prunes 94 % of two scans for a small, consistent gain.
M4 Pro (12 cores / 24 GB), release binaries from separate target dirs, base f7e2db3, 2 rounds x 3 iterations, sides alternated per round, machine otherwise idle (1-minute load never above the core count). Geomean of per-query ratios, per round, next to how much each binary differs from itself between rounds:
Every ratio is inside the same-binary band. The 1.026 for TPC-DS round 1 is one fast base sample (base differs from itself by 1.028; the branch is stable and round 2 is 0.997). Row counts are identical between base and branch for all 468 query/mode pairs.
Why it is flat: of the 745 hash joins in the 99 TPC-DS plans, 711 are inner or semi joins (covered by #25255) and only 26 are the types added here (Right 11, Left 10, LeftMark 3, RightMark 2). A
fact LEFT JOIN small_dimis planned as a Right join with the dimension as build side, so the transfer points at the small table. Only Q40 and Q80 (sales LEFT JOIN returns) gain a filter. Q80, pushdown mode:Q80 goes from 45.5 / 46.0 ms to 44.0 / 44.5 ms (both branch samples below both base samples); at SF1 the pruned scans are too small for more.
Observed, not yet explained: the two scans that prune sit under
Partitionedjoins;web_returns(Q80) andcatalog_returns(Q40) sit on the build side ofCollectLeftjoins, carry the same populated filter after execution, and prune nothing. An unpopulated dynamic filter istrue, so this is a missed optimization, never a wrong result. It looks like the build-side scan starting before the ancestor join has produced the filter; to be reproduced in isolation and tracked separately.What changes are included in this PR?
HashJoinExec::key_transferreplacessupports_key_transferand returnsExact(inner, semi),PruneOnly { preserved_child }(left, right, left mark, right mark, not null-aware) orNone.gather_filters_for_pushdown: forPruneOnlyonly the preserved-to-other key map is used, and a side that is not preserved receives nothing but transferred entries.handle_child_pushdown_result: forPruneOnlythe result is the preserved child's verdict per filter instead ofif_any. Without a transfer this equals the old result, since direct pushes only ever went to the preserved side.transfer_filter_across_keysno longer rewrites column-free filters, which the plain routing already handles.What is the testing strategy for this PR?
test_hashjoin_prune_only_transfer_differential: four join types x bothNullEqualityvalues x with and without a join filter xk = c,k IS NULL,NOT (k = c),k = c OR k IS NULL, over NULL, duplicate and one-sided keys. For every combination of which scans accept filters, the rows equal those of the plan where no scan accepts anything.test_hashjoin_prune_only_transfer_keeps_parent_filter: the copy reaches the other scan and does not remove theFilterExec; the preserved scan accepting it does.test_hashjoin_prune_only_transfer_negative_cases: non-preserved key, non-key column, mark column, full, anti and null-aware joins transfer nothing.test_hashjoin_prune_only_transfer_with_fetch: with a joinfetchevery row still comes from the join without a fetch.test_hashjoin_dynamic_filter_prune_only_through_left_join: an upper join's dynamic filter prunes the lower left join's other input, checked through scan metrics, with the NULL-extended row intact.join_dynamic_filter_transfer.slt: LEFT JOIN plan shape and results. One existing snapshot gains the transferred predicate.Are there any user-facing changes?
No new configuration.
EXPLAINmay show a filter on the non-preserved scan of a left, right or mark join.