Repository navigation
bench: add spill_views SQL benchmarks for spilling StringView/BinaryView columns - #25625
Merged
Merged
Conversation
Add sort and GROUP BY queries that spill StringView and BinaryView columns read from Parquet. Each query sets its own memory limit and target_partitions, so it spills in any environment. The `repeated` subgroup has a low-cardinality payload that shares one dictionary buffer; the `distinct` subgroup has all-distinct strings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
@kumarUjjawal since you reviewed #23565 previously would you mind taking a look here? |
adriangb
enabled auto-merge
September 22, 2026 17:57
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #25625 +/- ##
==========================================
- Coverage 82.42% 82.42% -0.01%
==========================================
Files 1140 1140
Lines 435586 435586
Branches 435586 435586
==========================================
- Hits 359042 359037 -5
- Misses 54828 54830 +2
- Partials 21716 21719 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
adriangb
added a commit
to pydantic/datafusion
that referenced
this pull request
Sep 23, 2026
…nment (apache#25634) ## Which issue does this PR close? - Follow-up to apache#25625. ## Rationale for this change Each `spill_views` query hardcodes its memory limit, and a benchmark-bot trigger comment can only set environment variables. So the only way to change a limit today is another DataFusion PR. At the current defaults every query spills on `main` and passes (q05, the GROUP BY, passed 30 of 30 executions at every limit from 96M to 192M). But apache#23565 fails q05 at 96M on every bot run (4 of 4), which aborts the whole suite run, so that PR gets no numbers. ## What changes are included in this PR? - `SPILL_VIEWS_LIMIT_REPEATED` (default `40M`) and `SPILL_VIEWS_LIMIT_DISTINCT` (default `96M`) set the limit of each subgroup, also as `--limit-repeated` / `--limit-distinct` on `benchmark_runner`. The defaults do not change. - The docs suggest `128M` as an override: q05 stops spilling at about `192M`, so larger values no longer measure the spill path. ## Are these changes tested? Ran the suite through `benchmark_runner` at the defaults and with `SPILL_VIEWS_LIMIT_DISTINCT=128M`. The suite's asserts read back the limit each query ran with, so a wrong override fails the run. ## Are there any user-facing changes? No, benchmarks only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Omega359
pushed a commit
to Omega359/arrow-datafusion
that referenced
this pull request
Oct 11, 2026
…iew columns (apache#25625) ## Which issue does this PR close? - Refers to apache#23564. - Precursor for apache#23565. This PR must merge first, so the benchmark bot can compare `main` and that PR with `run benchmark spill_views`. ## Rationale for this change apache#23565 changes how spilled `StringView` and `BinaryView` columns are compacted. No current benchmark shows the effect: - The `sort_tpch` string columns are either high-cardinality (`l_comment`) or have a dictionary buffer that is too small to be compacted (`l_shipinstruct`). - Most suites spill only when the environment sets a low memory limit. ## What changes are included in this PR? A new SQL benchmark suite `spill_views` (in `benchmarks/sql_benchmarks/spill_views/`) and a `bench.sh` entry for it. There is no change to the spill code. Each query reads 1M rows from a Parquet file that the suite writes with `COPY`. The data must come from Parquet, because then the views of a dictionary-encoded column point into one shared buffer. Each query sets its own memory limit and `target_partitions = 4`, so it spills in any environment. | Query | Subgroup | What it does | Memory limit | |---|---|---|---| | `q01_sort_string_1_distinct` | `repeated` | `ORDER BY` a shuffled key, `StringView` payload with 1 distinct value (64 bytes) | 40M | | `q02_sort_string_1000_distinct` | `repeated` | Same, with 1000 distinct values | 40M | | `q03_sort_binary_1000_distinct` | `repeated` | Same as q02, with a `BinaryView` payload | 40M | | `q04_sort_string_all_distinct` | `distinct` | Same, with all-distinct values (compaction cannot remove data) | 96M | | `q05_group_by_string_all_distinct` | `distinct` | `GROUP BY` an all-distinct string key (compaction cannot remove data) | 96M | The asserts check the row count, the number of distinct values, that the column is read as a view type, and that the memory limit and `target_partitions` settings are applied. ## What is the testing strategy for this PR? I ran the suite with `benchmark_runner` and with `./bench.sh run spill_views` (the `cargo bench --bench sql` path that the bot uses). All asserts pass and each query takes less than 150 ms per iteration. I also ran the same queries with `EXPLAIN ANALYZE`, on `main` and with this commit on top of apache#23565. All queries spill on both sides. Spilled bytes are from the sort or final aggregation operator. Times are the median of 40 iterations of `benchmark_runner` on a laptop (Apple M4 Pro, local SSD), with runs of both builds alternated: | Query | Spilled (`main`) | Spilled (PR) | Time (`main`) | Time (PR) | |---|---|---|---|---| | q01 sort, 1 distinct | 84.2 MB | 23.2 MB | 63.5 ms | 57.0 ms | | q02 sort, 1000 distinct | 84.2 MB | 30.6 MB | 57.0 ms | 65.3 ms | | q03 sort, 1000 distinct binary | 84.2 MB | 30.6 MB | 54.0 ms | 65.9 ms | | q04 sort, all distinct | 84.2 MB | 84.2 MB | 71.0 ms | 95.7 ms | | q05 group by, all distinct | 84.2 MB | 84.2 MB | 75.2 ms | 77.7 ms | The machine was under load, so the times are only approximate. On a local SSD, spill I/O is cheap. The benchmark bot will give better numbers. ## Are there any user-facing changes? No. This PR only adds a benchmark. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Omega359
pushed a commit
to Omega359/arrow-datafusion
that referenced
this pull request
Oct 11, 2026
…nment (apache#25634) ## Which issue does this PR close? - Follow-up to apache#25625. ## Rationale for this change Each `spill_views` query hardcodes its memory limit, and a benchmark-bot trigger comment can only set environment variables. So the only way to change a limit today is another DataFusion PR. At the current defaults every query spills on `main` and passes (q05, the GROUP BY, passed 30 of 30 executions at every limit from 96M to 192M). But apache#23565 fails q05 at 96M on every bot run (4 of 4), which aborts the whole suite run, so that PR gets no numbers. ## What changes are included in this PR? - `SPILL_VIEWS_LIMIT_REPEATED` (default `40M`) and `SPILL_VIEWS_LIMIT_DISTINCT` (default `96M`) set the limit of each subgroup, also as `--limit-repeated` / `--limit-distinct` on `benchmark_runner`. The defaults do not change. - The docs suggest `128M` as an override: q05 stops spilling at about `192M`, so larger values no longer measure the spill path. ## Are these changes tested? Ran the suite through `benchmark_runner` at the defaults and with `SPILL_VIEWS_LIMIT_DISTINCT=128M`. The suite's asserts read back the limit each query ran with, so a wrong override fails the run. ## Are there any user-facing changes? No, benchmarks only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Which issue does this PR close?
mainand that PR withrun benchmark spill_views.Rationale for this change
#23565 changes how spilled
StringViewandBinaryViewcolumns are compacted. No current benchmark shows the effect:sort_tpchstring columns are either high-cardinality (l_comment) or have a dictionary buffer that is too small to be compacted (l_shipinstruct).What changes are included in this PR?
A new SQL benchmark suite
spill_views(inbenchmarks/sql_benchmarks/spill_views/) and abench.shentry for it. There is no change to the spill code.Each query reads 1M rows from a Parquet file that the suite writes with
COPY. The data must come from Parquet, because then the views of a dictionary-encoded column point into one shared buffer. Each query sets its own memory limit andtarget_partitions = 4, so it spills in any environment.q01_sort_string_1_distinctrepeatedORDER BYa shuffled key,StringViewpayload with 1 distinct value (64 bytes)q02_sort_string_1000_distinctrepeatedq03_sort_binary_1000_distinctrepeatedBinaryViewpayloadq04_sort_string_all_distinctdistinctq05_group_by_string_all_distinctdistinctGROUP BYan all-distinct string key (compaction cannot remove data)The asserts check the row count, the number of distinct values, that the column is read as a view type, and that the memory limit and
target_partitionssettings are applied.What is the testing strategy for this PR?
I ran the suite with
benchmark_runnerand with./bench.sh run spill_views(thecargo bench --bench sqlpath that the bot uses). All asserts pass and each query takes less than 150 ms per iteration.I also ran the same queries with
EXPLAIN ANALYZE, onmainand with this commit on top of #23565. All queries spill on both sides. Spilled bytes are from the sort or final aggregation operator. Times are the median of 40 iterations ofbenchmark_runneron a laptop (Apple M4 Pro, local SSD), with runs of both builds alternated:main)main)The machine was under load, so the times are only approximate. On a local SSD, spill I/O is cheap. The benchmark bot will give better numbers.
Are there any user-facing changes?
No. This PR only adds a benchmark.
🤖 Generated with Claude Code