Group related conjuncts back together in the scan filter - #9877
joseph-isaacs wants to merge 2 commits into
Conversation
`FilterExpr` splits a filter on `AND` so each conjunct can be reordered and short-circuited on its own selectivity. That costs a separate decode of a column every time two conjuncts read it. Regroup the conjuncts after splitting, under a `ConjunctGrouping` selected at runtime by `VORTEX_CONJUNCT_GROUPING`: - `shared` (default) merges conjuncts sharing any field path, transitively via union-find, so `a > 5 AND a < b AND b = 2` becomes one group; - `same` merges only conjuncts whose referenced field paths are equal; - `none` keeps the previous behaviour. Grouping is by connected component, and both the groups and their members keep the original conjunct order. `shared` is the default on TPC-H sf1 evidence. Measured by round-robin interleaving the three settings (a sequential A/B/C sweep is worthless here: the same `none` configuration ran 1.76x faster from position 3 than position 1 on page-cache warming alone). Interleaved, q12 is faster in 8 of 8 rounds, median ratio 0.852; q1/q6/q19 sit within noise at 0.99-1.04. q12 is the query that has anything to group: its lineitem filter keeps `l_receiptdate > l_commitdate`, `l_shipdate < l_commitdate` and a `l_receiptdate` range, which chain into one group of 4 conjuncts down to 2. `same` finds nothing to do anywhere in TPC-H, because `find_between` already folds every same-column literal range into a single `Between` before the scan sees the filter. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HVk8ebAE2G3oJgMe1UaSfq
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Merging this PR will degrade performance by 9.71%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | random_i8[0.5] |
70.2 µs | 93.7 µs | -25.06% |
| ❌ | Simulation | decompress[u64, (4000, 1024)] |
70.5 µs | 85.7 µs | -17.71% |
| ❌ | WallTime | words_gather_scalar_avx2[65536] |
8.3 µs | 9.4 µs | -11.77% |
| ❌ | WallTime | mul_u32_nonnull_avx512 |
5.6 µs | 6.3 µs | -10.81% |
| ⚡ | Simulation | random_i16[0.8] |
95.4 µs | 77.2 µs | +23.65% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/conjunct-grouping-scan-builder-8sjfly (d641b86) with develop (6465462)
Footnotes
-
218 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (0.990x ➖, 0↑ 0↓)
No file size changes detected. |
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 / vortex-compact / ns (1.008x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.995x ➖, 1↑ 1↓)
duckdb / vortex-compact / ns (0.997x ➖, 0↑ 1↓)
duckdb / parquet / ns (0.992x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.987x ➖, 0↑ 1↓)
datafusion / vortex-compact / ns (1.000x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.945x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.967x ➖, 3↑ 2↓)
duckdb / vortex-compact / ns (0.966x ➖, 3↑ 1↓)
duckdb / parquet / ns (0.990x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Clickbench Sorted on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.000x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.994x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.015x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.989x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (0.977x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.979x ➖, 1↑ 0↓)
File Size Changes (200 files changed, +0.0% overall, 115↑ 85↓)
Totals:
|
Benchmarks: FineWeb S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.967x ➖, 1↑ 1↓)
datafusion / vortex-compact / ns (0.915x ➖, 2↑ 1↓)
datafusion / parquet / ns (1.148x ➖, 0↑ 2↓)
duckdb / vortex-file-compressed / ns (1.014x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.980x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.985x ➖, 1↑ 0↓)
|
Benchmarks: Appian on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-compact / ns (0.999x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.006x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.001x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.989x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Statistical and Population Genetics 📖Commits: PR How to read Verdict and Engines
duckdb / vortex-file-compressed / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.000x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.997x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.002x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.006x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.023x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.000x ➖, 2↑ 2↓)
duckdb / vortex-compact / ns (0.992x ➖, 2↑ 1↓)
duckdb / parquet / ns (0.999x ➖, 0↑ 1↓)
No file size changes detected. |
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.995x ➖, 1↑ 0↓)
datafusion / vortex-compact / ns (1.001x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.002x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.983x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (1.002x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.999x ➖, 0↑ 0↓)
No file size changes detected. |
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.998x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.002x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.995x ➖, 0↑ 1↓)
duckdb / vortex-compact / ns (0.990x ➖, 3↑ 1↓)
duckdb / parquet / ns (1.008x ➖, 2↑ 6↓)
No file size changes detected. |
Benchmarks: String Encoding 📖Commits: PR vortex / vortex-file-compressed / ms (0.999x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / % (1.000x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.977x ➖, 1↑ 1↓)
datafusion / vortex-compact / ns (0.965x ➖, 1↑ 2↓)
datafusion / parquet / ns (1.049x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (0.971x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (0.916x ➖, 2↑ 0↓)
duckdb / parquet / ns (0.996x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=10 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-compact / ns (1.052x ➖, 1↑ 4↓)
datafusion / parquet / ns (1.090x ➖, 0↑ 3↓)
duckdb / vortex-compact / ns (0.934x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.979x ➖, 0↑ 1↓)
|
Benchmarks: Compression 📖Commits: PR vortex / vortex-file-compressed / ns (0.984x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / bytes (1.000x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / ratio (0.990x ➖, 0↑ 0↓)
vortex / parquet / ns (0.999x ➖, 0↑ 0↓)
vortex / parquet / bytes (1.000x ➖, 0↑ 0↓)
vortex / arrow-ipc / ns (0.999x ➖, 0↑ 0↓)
vortex / arrow-ipc / bytes (1.000x ➖, 0↑ 0↓)
|
Benchmarks: Random Access 📖Commits: PR How to read Verdict and Engines
vortex / arrow-ipc / ns (0.908x ➖, 3↑ 0↓)
random-access / vortex-file-compressed / ns (1.000x ➖, 0↑ 0↓)
random-access / parquet / ns (0.967x ➖, 1↑ 0↓)
random-access / lance / ns (0.986x ➖, 0↑ 0↓)
|
CI's fmt job rejected the `assert_grouped` call in `disjoint_fields_are_never_grouped`. Reformatted with nightly-2026-09-10, the toolchain pinned as NIGHTLY_TOOLCHAIN in .github/workflows/ci.yml. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HVk8ebAE2G3oJgMe1UaSfq Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
38f66b6 to
d641b86
Compare
CI status on
|
| head | tree vs previous | CodSpeed verdict |
|---|---|---|
21050aa |
— | −11.36%: words_gather_scalar_avx2, mul_u32_nonnull_avx512 |
38f66b6 |
whitespace inside a #[cfg(test)] module |
−17.71%: decompress[u64, (4000, 1024)] |
d641b86 |
identical — git diff 38f66b6 d641b86 is empty, commit messages only |
−9.71%: random_i8[0.5], decompress[u64, …], words_gather_scalar_avx2, mul_u32_nonnull_avx512, plus random_i16[0.8] at +23.65% |
38f66b6 and d641b86 have byte-identical trees yet produce different regression sets — including an improvement of comparable magnitude to the "regressions".
Structurally these benchmarks cannot reach the changed code. They live in vortex-array (filter_fixed_width.rs, binary_ops.rs), vortex-buffer (collect_bool.rs) and encodings/runend (run_end_compress.rs). The dependency arrow runs vortex-layout → {vortex-array, vortex-buffer, vortex-runend}; none of those crates depends on vortex-layout, so none of them links FilterExpr at all. CodSpeed's own report carries both "Unknown Walltime execution environment detected" and "Different runtime environments detected".
The measurements that do exercise this change are the vx-bench q12 numbers in the PR description.
Both checks have now been re-run once (the push to d641b86 re-ran the full suite) and both reproduced, which is consistent with the above rather than with transient failure.
Generated by Claude Code
Summary
FilterExprsplits a filter onANDso each conjunct can be reordered and short-circuited on its own selectivity. That costs a separate decode of a column every time two conjuncts read it.This regroups the conjuncts after splitting, trading scheduling granularity for a single pass over the columns a group shares.
Changes
vortex-layout/src/scan/filter.rsgains aConjunctGrouping, selected at runtime byVORTEX_CONJUNCT_GROUPING:shared(default)a > 5 AND a < b AND b = 2becomes one groupsamenoneGrouping is by connected component, and both the groups and their members keep the original conjunct order.
FilterExpr::newbecomes fallible, since the field-path analysis can error;RepeatedScan::executepropagates that.Which queries this actually affects
Expression::optimize_recursivealready runsfind_between, which folds every same-column literal range (</<=paired with>/>=) into a singleBetweenbefore the scan sees the filter. So the textbooka > 5 AND a < 10case is handled upstream, andsamefinds nothing to group anywhere in TPC-H. It is kept as a selectable option to make that distinction explicit and measurable.What survives for
sharedis TPC-H q12, whose lineitem filter reaches the scan as:The three date conjuncts chain into one group: 4 conjuncts down to 2. (q19's part-side merges 2→1, but
partis small enough that it does not show.)Benchmark results
Full
action/bench-allrun on this commit. q12 is the query with anything to group, and it improves on every Vortex target, on both engines, at both NVMe scale factors, while the Parquet controls stay flat — Parquet does not go through the Vortex scan filter, so a real effect should show exactly this asymmetry.q12 hot, PR vs base:
datafusion:vortex-file-compresseddatafusion:vortex-compactduckdb:vortex-file-compressedduckdb:vortex-compactdatafusion:parquet(control)duckdb:parquet(control)The effect grows with scale factor, as expected: the larger the row count, the more the duplicated per-conjunct column decode costs. The S3 variants show the same direction and larger magnitudes, but those suites are flagged
environment too noisy, so they are directional only.No regressions. All fourteen benchmark suites report
No clear signal, with Vortex geomeans at or slightly below 1.0 — including TPC-H SF=1/SF=10, TPC-DS, ClickBench (plain and sorted), FineWeb, Appian, PolarSignals, StatPopGen, random-access, string and compression. Scattered ±5-18% movements on non-q12 queries fall on queries where the grouping provably cannot act, and run in both directions; that is the noise the runner's low-confidence verdict reflects.Local measurement, and why it needed care
A sequential A/B sweep is worthless on a small box: the same
noneconfiguration measured 1.76x faster from position 3 than position 1, purely on page-cache warming, and anone-vs-nonecontrol showed ±27% per-query swings. Measuring by round-robin interleaving withnone, so drift is common-mode, q12 came out faster in 8/8 rounds (median ratio 0.852) and again in 7/8 on an independent re-check (median 0.911, pooled min 0.850). The CI run above is the clean confirmation.Testing
cargo clippy -p vortex-layout --all-targets --all-features -- -D warnings— clean.cargo test -p vortex-layout --lib— 306 passed, 0 failed.New tests in
scan::filter::testscover grouping a range over one field,samekeeping a partial field overlap apart,sharedgrouping transitively across a bridging conjunct, and disjoint fields never grouping under any setting.🤖 Generated with Claude Code
https://claude.ai/code/session_01HVk8ebAE2G3oJgMe1UaSfq