Refactor InListExpr into static-filter modules#21649
Refactor InListExpr into static-filter modules#21649geoffreyclaude wants to merge 1 commit intoapache:mainfrom
Conversation
b3fb108 to
f03a2d1
Compare
|
run benchmark in_list |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/in_list_static_filter (f03a2d1) to 93ae1b8 (merge-base) diff File an issue against this benchmark runner |
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
WAT -- thsoe are some nice numbers. I wil try and find time to review this PR |
That's unexpected, it was supposed to be a pure refactor PR. I suspect it's doing more than what it says, I'll review it more carefully. EDIT: I’ve rebased this PR since then. It’s now just the single refactor commit, so it should be reviewed as a mechanical/module-structure cleanup. |
f03a2d1 to
b96f00d
Compare
b96f00d to
3fa6673
Compare
|
run benchmark in_list |
|
run benchmark in_list_strategy |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/in_list_static_filter (3fa6673) to 5a427cb (merge-base) diff File an issue against this benchmark runner |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/in_list_static_filter (3fa6673) to 5a427cb (merge-base) diff File an issue against this benchmark runner |
3fa6673 to
fc15088
Compare
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Moves the existing primitive and generic static-filter implementations into dedicated modules without changing the underlying algorithms. Keeps ArrayStaticFilter as the generic fallback and preserves the pre-existing result construction path.
fc15088 to
539f998
Compare
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Which issue does this PR close?
Rationale for this change
This PR is a preparatory refactor for #19390.
InListExprcurrently mixes expression evaluation, constant-list filter construction, primitive-vs-fallback dispatch, and generic fallback execution in one file.This PR extracts the existing static-filter code into smaller internal modules while preserving the current behavior.
Review guide
This PR is intended to be a move/extract-only refactor. The main review question is whether the existing
InListExprstatic-filter behavior has been preserved while making the internal structure easier to follow.In particular:
in_list.rsstill ownsInListExprconstruction and evaluationprimitive_filter.rscontains the existing primitive numeric static-filter implementations moved out ofin_list.rsarray_static_filter.rscontains the existingArrayStaticFilterfallback moved out ofin_list.rsstrategy.rscontains the existing dispatch between primitive filters andArrayStaticFilterstatic_filter.rsdefines the shared internal trait used by those extracted piecesNo user-facing behavior or API changes are intended in this PR.
Are these changes tested?
Yes. I validated this PR with:
cargo fmt --allcargo clippy -p datafusion-physical-expr --all-targets --all-features -- -D warningscargo test -p datafusion-physical-expr --all-featuresAre there any user-facing changes?
No. This is an internal refactor only.