Extract dynamic filter expression composition from `SharedBuildAccumulator::build_filter` by kosiew · Pull Request #23370 · apache/datafusion

kosiew · 2026-07-07T10:12:32Z

Which issue does this PR close?

Part of Centralize dynamic-filter expression policy outside shared accumulator finalization #22772

Rationale for this change

SharedBuildAccumulator::build_filter currently combines lifecycle coordination with dynamic-filter expression construction for both collect-left and partitioned execution paths. This interleaves synchronization logic with expression composition, making the expression policy harder to evolve independently.

This change extracts the expression assembly into a dedicated private helper so that the accumulator remains responsible for coordinating finalization and applying dynamic filter updates, while the helper owns expression construction and fallback policy. The refactoring is intended to preserve existing dynamic-filter semantics.

What changes are included in this PR?

Introduce a private DynamicFilterExprComposer responsible for finalize-time dynamic-filter expression composition.
Add FilterComposition to distinguish between updating the dynamic filter and intentionally performing no update.
Move collect-left expression construction into the composer.
Move partitioned expression construction, including partition routing, fallback handling, and CASE expression generation, into the composer.
Extract helper methods for composing reported partition branches and building the routing modulo expression.
Simplify SharedBuildAccumulator::build_filter so it delegates expression construction to the composer and only applies updates when required.
Preserve existing error handling for invalid finalize states such as pending or unexpected partition status.

Are these changes tested?

No tests are included in this PR.

This change is a refactoring intended to preserve existing behavior. Existing tests covering hash-join dynamic filtering are expected to continue to pass unchanged.

Are there any user-facing changes?

No.

LLM-generated code disclosure

This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed.

- Added private components: - DynamicFilterExprComposer - FilterComposition variants (NoUpdate, Update) - Moved CollectLeft and Partitioned expression policies into the composer. - Updated SharedBuildAccumulator::build_filter to be a thin delegate with dynamic_filter.update integration.

…bounds - Built lazy routing modulo expr only when CASE is needed. - Deduplicated CaseExpr construction using partition_case. - Extracted reported-partition branch assembly into compose_reported_partition_branch. - Removed trivial private constructor; utilized struct literal instead.

…ntation

kosiew · 2026-07-07T10:12:53Z

run benchmark hj

adriangbot · 2026-07-07T10:15:44Z

🤖 Benchmark running (GKE) | trigger
Instance: c4a-highmem-16 (12 vCPU / 65 GiB) | Linux bench-c4902655376-890-t5wpf 6.12.85+ #1 SMP Mon May 11 08:17:35 UTC 2026 aarch64 GNU/Linux

CPU Details (lscpu)

Architecture:                            aarch64
CPU op-mode(s):                          64-bit
Byte Order:                              Little Endian
CPU(s):                                  16
On-line CPU(s) list:                     0-15
Vendor ID:                               ARM
Model name:                              Neoverse-V2
Model:                                   1
Thread(s) per core:                      1
Core(s) per cluster:                     16
Socket(s):                               -
Cluster(s):                              1
Stepping:                                r0p1
BogoMIPS:                                2000.00
Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache:                               1 MiB (16 instances)
L1i cache:                               1 MiB (16 instances)
L2 cache:                                32 MiB (16 instances)
L3 cache:                                80 MiB (1 instance)
NUMA node(s):                            1
NUMA node0 CPU(s):                       0-15
Vulnerability Gather data sampling:      Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit:             Not affected
Vulnerability L1tf:                      Not affected
Vulnerability Mds:                       Not affected
Vulnerability Meltdown:                  Not affected
Vulnerability Mmio stale data:           Not affected
Vulnerability Reg file data sampling:    Not affected
Vulnerability Retbleed:                  Not affected
Vulnerability Spec rstack overflow:      Not affected
Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
Vulnerability Spectre v2:                Mitigation; CSV2, BHB
Vulnerability Srbds:                     Not affected
Vulnerability Tsa:                       Not affected
Vulnerability Tsx async abort:           Not affected
Vulnerability Vmscape:                   Not affected

Comparing dynamic-filter-02-22772 (63cb7b6) to d09a52a (merge-base) diff using: hj
Results will be posted here when complete

File an issue against this benchmark runner

kosiew · 2026-07-07T10:20:41Z

run benchmark sql

 env:
     BENCH_NAME: hj

adriangbot · 2026-07-07T10:23:33Z

🤖 Benchmark running (GKE) | trigger
Instance: c4a-highmem-16 (12 vCPU / 65 GiB) | Linux bench-c4902719872-891-vljr5 6.12.85+ #1 SMP Mon May 11 08:17:35 UTC 2026 aarch64 GNU/Linux

CPU Details (lscpu)

Architecture:                            aarch64
CPU op-mode(s):                          64-bit
Byte Order:                              Little Endian
CPU(s):                                  16
On-line CPU(s) list:                     0-15
Vendor ID:                               ARM
Model name:                              Neoverse-V2
Model:                                   1
Thread(s) per core:                      1
Core(s) per cluster:                     16
Socket(s):                               -
Cluster(s):                              1
Stepping:                                r0p1
BogoMIPS:                                2000.00
Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache:                               1 MiB (16 instances)
L1i cache:                               1 MiB (16 instances)
L2 cache:                                32 MiB (16 instances)
L3 cache:                                80 MiB (1 instance)
NUMA node(s):                            1
NUMA node0 CPU(s):                       0-15
Vulnerability Gather data sampling:      Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit:             Not affected
Vulnerability L1tf:                      Not affected
Vulnerability Mds:                       Not affected
Vulnerability Meltdown:                  Not affected
Vulnerability Mmio stale data:           Not affected
Vulnerability Reg file data sampling:    Not affected
Vulnerability Retbleed:                  Not affected
Vulnerability Spec rstack overflow:      Not affected
Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
Vulnerability Spectre v2:                Mitigation; CSV2, BHB
Vulnerability Srbds:                     Not affected
Vulnerability Tsa:                       Not affected
Vulnerability Tsx async abort:           Not affected
Vulnerability Vmscape:                   Not affected

Comparing dynamic-filter-02-22772 (63cb7b6) to d09a52a (merge-base) diff using: sql
Results will be posted here when complete

File an issue against this benchmark runner

adriangbot · 2026-07-07T10:32:46Z

🤖 Benchmark completed (GKE) | trigger

Instance: c4a-highmem-16 (12 vCPU / 65 GiB)

CPU Details (lscpu)

Architecture:                            aarch64
CPU op-mode(s):                          64-bit
Byte Order:                              Little Endian
CPU(s):                                  16
On-line CPU(s) list:                     0-15
Vendor ID:                               ARM
Model name:                              Neoverse-V2
Model:                                   1
Thread(s) per core:                      1
Core(s) per cluster:                     16
Socket(s):                               -
Cluster(s):                              1
Stepping:                                r0p1
BogoMIPS:                                2000.00
Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache:                               1 MiB (16 instances)
L1i cache:                               1 MiB (16 instances)
L2 cache:                                32 MiB (16 instances)
L3 cache:                                80 MiB (1 instance)
NUMA node(s):                            1
NUMA node0 CPU(s):                       0-15
Vulnerability Gather data sampling:      Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit:             Not affected
Vulnerability L1tf:                      Not affected
Vulnerability Mds:                       Not affected
Vulnerability Meltdown:                  Not affected
Vulnerability Mmio stale data:           Not affected
Vulnerability Reg file data sampling:    Not affected
Vulnerability Retbleed:                  Not affected
Vulnerability Spec rstack overflow:      Not affected
Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
Vulnerability Spectre v2:                Mitigation; CSV2, BHB
Vulnerability Srbds:                     Not affected
Vulnerability Tsa:                       Not affected
Vulnerability Tsx async abort:           Not affected
Vulnerability Vmscape:                   Not affected

Details

Comparing HEAD and dynamic-filter-02-22772
--------------------
Benchmark hj.json
--------------------
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query                                                               ┃                               HEAD ┃            dynamic-filter-02-22772 ┃        Change ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 1_density=1_prob_hit=1_25*1.5M                               │        1.98 / 2.49 ±0.60 / 3.61 ms │        2.12 / 2.38 ±0.43 / 3.23 ms │     no change │
│ QQuery 2_density=0.026_prob_hit=1_25*1.5M                           │        4.21 / 4.45 ±0.16 / 4.71 ms │        4.21 / 4.42 ±0.12 / 4.55 ms │     no change │
│ QQuery 3_density=1_prob_hit=1_100K*60M                              │     87.11 / 88.39 ±0.85 / 89.48 ms │     87.70 / 88.01 ±0.35 / 88.49 ms │     no change │
│ QQuery 4_density=1_prob_hit=0.1_100K*60M                            │ 193.78 / 218.11 ±23.36 / 246.38 ms │  195.58 / 198.07 ±2.42 / 202.38 ms │ +1.10x faster │
│ QQuery 5_density=0.75_prob_hit=1_100K*60M                           │ 729.20 / 743.41 ±10.30 / 757.82 ms │ 724.20 / 740.10 ±14.51 / 764.71 ms │     no change │
│ QQuery 6_density=0.75_prob_hit=0.1_100K*60M                         │ 280.06 / 299.17 ±18.79 / 326.29 ms │ 279.65 / 289.83 ±12.26 / 307.44 ms │     no change │
│ QQuery 7_density=0.5_prob_hit=1_100K*60M                            │ 708.45 / 736.78 ±19.21 / 764.43 ms │ 714.26 / 737.52 ±14.15 / 755.58 ms │     no change │
│ QQuery 8_density=0.5_prob_hit=0.1_100K*60M                          │ 255.14 / 268.44 ±19.32 / 306.72 ms │ 257.06 / 281.35 ±21.87 / 312.69 ms │     no change │
│ QQuery 9_density=0.2_prob_hit=1_100K*60M                            │ 742.22 / 773.51 ±42.07 / 856.54 ms │ 744.12 / 760.29 ±16.69 / 792.40 ms │     no change │
│ QQuery 10_density=0.2_prob_hit=0.1_100K*60M                         │ 260.18 / 283.34 ±24.63 / 319.76 ms │ 256.42 / 278.13 ±18.88 / 307.52 ms │     no change │
│ QQuery 11_density=0.1_prob_hit=1_100K*60M                           │ 742.12 / 774.75 ±25.06 / 811.33 ms │ 725.95 / 747.59 ±20.30 / 782.29 ms │     no change │
│ QQuery 12_density=0.1_prob_hit=0.1_100K*60M                         │ 261.06 / 287.73 ±20.84 / 314.38 ms │ 255.56 / 284.19 ±16.74 / 302.50 ms │     no change │
│ QQuery 13_density=0.01_prob_hit=1_100K*60M                          │  756.33 / 767.62 ±9.17 / 784.06 ms │ 746.71 / 765.32 ±14.28 / 790.59 ms │     no change │
│ QQuery 14_density=0.01_prob_hit=0.1_100K*60M                        │ 286.71 / 317.89 ±37.96 / 386.88 ms │ 269.72 / 303.01 ±26.84 / 345.12 ms │     no change │
│ QQuery 15_density=0.2_prob_hit=0.1_100K_(20%_dups)*60M              │ 264.77 / 279.79 ±17.52 / 311.00 ms │ 257.24 / 276.25 ±26.54 / 327.16 ms │     no change │
│ QQuery 16_density=1_prob_hit=1_25*1.5M_RightSemi                    │        2.47 / 2.59 ±0.09 / 2.71 ms │        2.35 / 2.54 ±0.15 / 2.76 ms │     no change │
│ QQuery 17_density=1_prob_hit=1_100K*60M_RightSemi                   │  89.43 / 102.17 ±20.71 / 143.30 ms │  89.90 / 100.92 ±13.70 / 125.55 ms │     no change │
│ QQuery 18_density=1_prob_hit=0.1_100K*60M_RightSemi                 │  100.82 / 104.12 ±5.52 / 115.14 ms │  100.57 / 101.35 ±0.90 / 103.12 ms │     no change │
│ QQuery 19_density=1_prob_hit=1_25*1.5M_RightAnti                    │        1.97 / 2.04 ±0.04 / 2.09 ms │        2.10 / 2.19 ±0.09 / 2.36 ms │  1.07x slower │
│ QQuery 20_density=1_prob_hit=1_100K*60M_RightAnti                   │     80.63 / 81.74 ±0.81 / 83.03 ms │     81.76 / 82.78 ±0.92 / 84.06 ms │     no change │
│ QQuery 21_density=1_prob_hit=0.1_100K*60M_RightAnti                 │  111.59 / 112.67 ±1.38 / 115.33 ms │  110.31 / 111.98 ±1.49 / 114.40 ms │     no change │
│ QQuery 22_density=1_prob_hit=0.01_100K_(fanout_100)*60M_RightSemi   │     91.65 / 92.97 ±0.87 / 94.21 ms │     88.50 / 92.43 ±2.41 / 95.24 ms │     no change │
│ QQuery 23_density=1_prob_hit=1_32K_(fanout~78)*2.3M_long_keys_count │  165.83 / 166.67 ±1.00 / 168.55 ms │  167.47 / 167.89 ±0.23 / 168.09 ms │     no change │
└─────────────────────────────────────────────────────────────────────┴────────────────────────────────────┴────────────────────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary                      ┃           ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD)                      │ 6510.83ms │
│ Total Time (dynamic-filter-02-22772)   │ 6418.57ms │
│ Average Time (HEAD)                    │  283.08ms │
│ Average Time (dynamic-filter-02-22772) │  279.07ms │
│ Queries Faster                         │         1 │
│ Queries Slower                         │         1 │
│ Queries with No Change                 │        21 │
│ Queries with Failure                   │         0 │
└────────────────────────────────────────┴───────────┘

Resource Usage

hj — base (merge-base)

Metric	Value
Wall time	35.0s
Peak memory	2.7 GiB
Avg memory	1.3 GiB
CPU user	329.2s
CPU sys	11.1s
Peak spill	0 B

hj — branch

Metric	Value
Wall time	35.0s
Peak memory	2.9 GiB
Avg memory	1.3 GiB
CPU user	328.7s
CPU sys	11.1s
Peak spill	0 B

File an issue against this benchmark runner

…f a vector

kosiew added 3 commits July 7, 2026 18:03

feat: refactor PartitionBranch to use struct variant for Real represe…

63cb7b6

…ntation

github-actions Bot added the physical-plan Changes to the physical-plan crate label Jul 7, 2026

fix: update compose_partitioned to accept a slice reference instead o…

be1b878

…f a vector

apache deleted a comment from adriangbot Jul 7, 2026

kosiew marked this pull request as ready for review July 7, 2026 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Extract dynamic filter expression composition from `SharedBuildAccumulator::build_filter`#23370

Extract dynamic filter expression composition from `SharedBuildAccumulator::build_filter`#23370
kosiew wants to merge 4 commits into
apache:mainfrom
kosiew:dynamic-filter-02-22772

kosiew commented Jul 7, 2026

Uh oh!

kosiew commented Jul 7, 2026

Uh oh!

adriangbot commented Jul 7, 2026

Uh oh!

kosiew commented Jul 7, 2026

Uh oh!

adriangbot commented Jul 7, 2026

Uh oh!

adriangbot commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

kosiew commented Jul 7, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

LLM-generated code disclosure

Uh oh!

kosiew commented Jul 7, 2026

Uh oh!

adriangbot commented Jul 7, 2026

Uh oh!

kosiew commented Jul 7, 2026

Uh oh!

adriangbot commented Jul 7, 2026

Uh oh!

adriangbot commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants