Make partition expression evaluation stateless#3664
Open
dossett wants to merge 3 commits into
Open
Conversation
dossett
marked this pull request as ready for review
July 15, 2026 17:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I used CODEX to analyze this problem and create this PR. I've reviewed the code and tests and stand by them. This summary is written completely by a human (me) other than very light copy editing by an LLM.
Currently, because
_ExpressionEvaluatorcan't be safely shared one is created per file. That's expensive relative to actually evaluating the expression. This change makes_ExpressionEvaluatorshareable by moving the mutable parts into a new_ExpressionEvaluationVisitorwhich stays local to the per-file work of evaluation.These benchmarks are showing improvements in sub-second times, but in the production workloads I tested this on it did translate to real wall clock improvement. But irrespective of performance gains the new
_ExpressionEvaluatoris safer since nothing was preventing its accidental concurrent reuse in the future.CODEX-generated summary follows:
Partition pruning currently binds the same projected expression for every data file because
_ExpressionEvaluatorstores the current record as mutable instance state. That repeated preparation is expensive, but sharing the existing evaluator across workers would allow concurrent calls to mix records.This separates preparation from per-record evaluation.
_ExpressionEvaluatorbinds the expression once, while every call creates a private_ExpressionEvaluationVisitorcontaining that call's record. Manifest planning can therefore prepare one evaluator per partition spec and safely share it across manifests and workers. This provides the construction-reuse benefit targeted by #3656 without depending on files within a manifest remaining sequential, and it also benefits one-file manifests.Summary
Performance
I compared this branch with
mainusing the partition-evaluator workload in this PR. It evaluates 1,000 files with two identity partition fields and a 15-leaf predicate modeling five(event_day range AND region_id)branches. Timings are medians from seven samples of five iterations.mainThe improvement comes from binding the projected partition expression once per partition spec instead of once per file. This is an isolated partition-pruning benchmark rather than an end-to-end scan-planning measurement.
Testing
pytest tests/expressions/test_visitors.py tests/table/test_partition_evaluator_planning.py tests/table/test_init.py(200 passed)pytest tests/benchmark/test_partition_evaluator_benchmark.py -m benchmark(2 passed)