Repository navigation
Try reduce_parent rules after execute_parent rewrites the current node - #11310
Conversation
Merging this PR will not alter performance2 warnings
Comparing Footnotes
|
136e3dc to
a0cf5e9
Compare
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (1.021x ➖, 1↑ 2↓)
|
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.069x ➖, 0↑ 3↓)
datafusion / parquet / ns (0.996x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.003x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.007x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.994x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.008x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.000x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.995x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.995x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.993x ➖, 1↑ 0↓)
|
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.000x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.004x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.997x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.999x ➖, 0↑ 1↓)
|
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.993x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.991x ➖, 0↑ 1↓)
duckdb / parquet / ns (0.993x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.082x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.024x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.958x ➖, 1↑ 0↓)
duckdb / parquet / ns (1.026x ➖, 0↑ 1↓)
|
Benchmarks: TPC-DS SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.996x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.992x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.994x ➖, 0↑ 0↓)
|
While a scalar function waits on the executor stack for an input, execution can rewrite that input into an encoding whose parent-reduce rules now match. The optimizer only saw the input as it was before execution, so those rules never ran. A slice over a run-end array executes into the run-end array, but `RunEndScalarFnRule` never pushed the suspended between into the run values, so every row was decoded and compared. After a child under a suspended parent is rewritten, put it back into the parent and try its parent-reduce rules, session kernels first. A rewrite replaces the parent; otherwise the child is detached again. Both are in place when the parent is uniquely owned. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018SWbEuVduQbmfuXAPx4p46 Signed-off-by: Robert Kruszewski <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018SWbEuVduQbmfuXAPx4p46 Signed-off-by: Robert Kruszewski <[email protected]>
a0cf5e9 to
3a4ded4
Compare
## Summary Stacked on #11310. Until that merges, this PR shows only the last commit. `Lower` and `Strpos` decode their inputs inside `execute`, in a nested executor. When the input is a slice or a mask over a run-end or chunked array, the function gets a canonical array and runs once per row, even when the encoding could have pushed it into its values. #11310 lets the executor retry parent-reduce rules once a child is rewritten, but only for functions that hand their inputs back to the executor through `execute_step`. This continues the `execute_step` migration from #11100 and #11154 for `Lower` and `Strpos`. ## Changes - `Lower::execute_step` and `Strpos::execute_step` request their inputs with `require_input!(args, i => AnyColumnar)`, as `Like` and `Between` do, then call `execute`. - Tests: `lower`'s `preserves_runs_and_dictionary` gets a sliced case, and `strpos` gets `preserves_runs_of_a_slice`. Both check that a function over a sliced run-end input stays run-end encoded. Both fail without this change, and both fail without #11310. Checks: - `cargo test -p vortex-array` passes. - `cargo clippy -p vortex-array --all-targets --all-features -- -D warnings` passes. - `cargo +nightly-2026-09-10 fmt -p vortex-array --check` passes. Benchmark numbers will follow. No existing benchmark covers scalar functions over sliced or masked encoded inputs. This change was written with help from an AI coding agent and I haven't reviewed it yet. --------- Signed-off-by: Robert Kruszewski <[email protected]> Signed-off-by: Alfonso Subiotto <[email protected]> Co-authored-by: Robert Kruszewski <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: Amp <[email protected]> Co-authored-by: Robert Kruszewski <[email protected]>
This changes is meant to catch and apply interleavings of execute and parent rules that otherwise have no chance to interact. Practical example of this is
Between(Slice(RunEnd))combination. RunEnd has optimisations for both Slice and Between, however, Slice is an execute rule (since it pushes the slice to the values and needs to do binary search on runs) and between is an reduce rule. After Slice runs and reduces toBetween(RunEnd)we never apply the reduce parent rule that pushesBetweento the values of RunEnd.With this change we try to apply rule after execute_parent is done.
This is likely a lot better once #11091 and #11092 merge where we can cheaply know if there's any rule that might apply