Skip to content

Add uncompressed_size_in_bytes to the preview edition - #10922

Merged
joseph-isaacs merged 11 commits into
developfrom
ji/jolly-wozniak-brx34l
Oct 29, 2026
Merged

joseph-isaacs merged 11 commits into
developfrom
ji/jolly-wozniak-brx34l

Conversation

@joseph-isaacs

@joseph-isaacs joseph-isaacs commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Second of a stack: 1) uncompressed size fixes (#10925) → 2) this PR → 3) record the stat in the file footer (#10923).

File statistics only record aggregates that the write's enabled editions include. vortex.uncompressed_size_in_bytes is in no edition, so the footer can't record a field's uncompressed size. This PR adds it to the draft preview2026.08.0 edition, so writers can opt in by enabling preview. Frozen core editions are unchanged.

Changes

  • vortex-edition: preview2026.08.0 adds EditionMember::aggregate("vortex.uncompressed_size_in_bytes").
  • vortex/editions/preview/preview2026.08.0.toml: regenerated with cargo run -p xtask -- generate-editions.

Checks run: cargo test -p vortex-edition, cargo run -p xtask -- check-editions (frozen records preserved).

🤖 Generated with Claude Code

https://claude.ai/code/session_01UeniBAJcn1sFtocvffo36c

Lets file statistics record the exact uncompressed size of each field
when the preview edition is enabled.

Signed-off-by: Claude <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01UeniBAJcn1sFtocvffo36c
…d size

A constant Utf8/Binary array with a value too long to inline counted that
value's bytes once, even for the exact size. The exact size counts the bytes
every row references, as the canonical VarBinView path does, so a constant
must count it once per row; only the inexact size counts the single buffer
canonicalization holds. A dictionary whose codes are constant executes into
a constant, so its exact size undercounted too.

Signed-off-by: Claude <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01UeniBAJcn1sFtocvffo36c
@codspeed

codspeed Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 30.79%

2 warnings

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚠️ 1 benchmark is dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚡ 8 improved benchmarks
✅ 2296 untouched benchmarks

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ CPU Simulation filter_powerlaw_by_random[10000] 421.1 ns 142.2 ns ×3
⚡ CPU Simulation bench_compare_varbin[(50000, 2)] 162.3 µs 133.2 µs +21.83%
⚡ CPU Simulation bench_compare_varbin[(50000, 64)] 164.1 µs 135 µs +21.58%
⚡ CPU Simulation encode_primitives[u8, (2000, 4)] 163.5 µs 134.7 µs +21.32%
⚡ CPU Simulation bench_compare_sliced_dict_varbinview[(1000, 10000)] 89.1 µs 78.1 µs +14.06%
⚡ CPU Simulation bench_compare_sliced_dict_varbinview[(7500, 10000)] 227.4 µs 200.5 µs +13.4%
⚡ CPU Simulation bench_compare_sliced_dict_varbinview[(10000, 10000)] 233.6 µs 206.7 µs +13%
⚡ Walltime starts_with_avx512[64, VarBin] 58.2 µs 52.9 µs +10.11%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ji/jolly-wozniak-brx34l (92664ac) with develop (1d09649)1

Open in CodSpeed

Footnotes

  1. No successful run was found on develop (2eae764) during the generation of this report, so 1d09649 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

The inexact size measures the bytes buffers hold, which can be smaller
than the exact size when rows share a value, so caching it in the stat
slot as an upper bound was wrong. An inexact query also reused a cached
exact size, so its answer depended on what had been computed before.
The inexact size now neither reads nor writes the slot, which holds only
the exact size.

Signed-off-by: Claude <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01UeniBAJcn1sFtocvffo36c
Lets file statistics record the exact uncompressed size of each field
when the preview edition is enabled.

Signed-off-by: Claude <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01UeniBAJcn1sFtocvffo36c
@joseph-isaacs
joseph-isaacs changed the base branch from develop to ji/uncompressed-size-fixes October 28, 2026 13:35
…size-fixes

An earlier merge kept the restacked tree with -s ours, which recorded the
old develop-based history as merged. Merging the up-to-date base on top
then dropped develop's newer changes. This brings the tree back to the
base plus this branch's edition change.

Signed-off-by: Claude <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01UeniBAJcn1sFtocvffo36c
Base automatically changed from ji/uncompressed-size-fixes to develop October 28, 2026 13:47
@joseph-isaacs
joseph-isaacs enabled auto-merge (squash) October 29, 2026 15:41
@joseph-isaacs
joseph-isaacs merged commit 2a9c762 into develop Oct 29, 2026
81 checks passed
@joseph-isaacs
joseph-isaacs deleted the ji/jolly-wozniak-brx34l branch October 29, 2026 15:48
joseph-isaacs added a commit that referenced this pull request Oct 29, 2026
…0923)

## Summary

Last of a stack. The first two PRs have merged: #10925
fixed the uncompressed size, and #10922 added
`vortex.uncompressed_size_in_bytes` to the preview edition.

The default file statistics now include each field's uncompressed size
in bytes, in both the nested statistics (`entry_sets`/`partials`) and
the legacy `ArrayStats` statistics. Both are gated by the existing
edition check, so the size is written only when the write's enabled
editions include `vortex.uncompressed_size_in_bytes` (currently the
preview edition) or when editions are disabled. Default writes, which
enable only the core editions, don't store it, and their files are
byte-for-byte the same size as before.

DataFusion doesn't use the stat yet: given it as column and table byte
sizes, its join selection picked worse plans (see Benchmarks below).

## Changes

- `vortex-layout` `file_stats`:
- `default_aggregate_fns` (renamed from `default_pruning_aggregate_fns`,
since it no longer returns only pruning aggregates) adds
`UncompressedSizeInBytes`. Explicit `with_file_statistics` lists are
unchanged.
- The legacy statistics now go through the same `allows_aggregate`
filter as the nested ones, and an explicitly requested aggregate the
context forbids fails the write in either layout. Every other default
aggregate is already in the core editions, so this only changes whether
the uncompressed size is recorded.
- `vortex-datafusion`: column `byte_size` and table `total_byte_size`
stay `Absent` rather than coming from `Stat::UncompressedSizeInBytes`. A
doc comment on `stats_set_to_df` gives the reasons (below), and a test
checks that a stats set holding the size still converts to an absent
`byte_size`.
- `vortex-duckdb`: DuckDB's `max_string_length` comes from this stat,
and it was cast with `as u32`. A column larger than 4 GiB wrapped into a
bound *smaller* than its longest string. Such a size now gives no bound.
- Tests:
- `vortex-layout`: an rstest over allowed/forbidden contexts checks that
both layouts record the size only when allowed;
`forbidden_aggregates_are_gated` now expects forbidden defaults to be
left out of the legacy stats too.
- `vortex-file`: `uncompressed_size_follows_editions` writes a file with
core only, core + preview, and editions disabled, and checks the nested
and legacy footer entries in each case.
- `vortex-datafusion`: `uncompressed_size_is_not_reported_as_byte_size`.

## API Changes

Files written with the preview edition (or with editions disabled) carry
the uncompressed size in both footer layouts. Default writes are
unchanged.

## Benchmarks

An earlier revision passed the stat to DataFusion. `action/bench-all`
showed DataFusion on Vortex 1.25× slower on TPC-DS SF=1 (46 queries
slower) and 1.16× slower on TPC-H SF=1, with Parquet and DuckDB
unchanged, and 30 TPC-DS SQL logic test plans changed join order. This
revision keeps the stat out of DataFusion and restores the plans;
benchmarks are re-running on it.

### Why DataFusion gets no byte size

The sizes are accurate: TPC-H q4's filtered `orders` build side was
estimated at 1.5 MB and produced 1.34 MB, and they are close to what
Parquet reports (2.07 MB against 1.70 MB for the 7 `supplier` columns q2
reads; the difference is the 16-byte `Utf8View` header). The regression
comes from two things in DataFusion 55:

1. **Join selection threshold.** `JoinSelection` picks `CollectLeft`
below `hash_join_single_partition_threshold` (1 MiB) when a byte size is
known, and below `hash_join_single_partition_threshold_rows` (128Ki
rows) otherwise. Without sizes, 57K–150K row build sides passed the row
limit; with sizes, the same 1–7 MB build sides fail the byte limit and
become partitioned joins that repartition the probe side (all 3.79M
`lineitem` rows in q4). With a 4 MiB threshold, TPC-H q2, q4 and q22
plan exactly as on `develop`.
2. **`ProjectionExprs::project_statistics`.** It keeps the input's
`total_byte_size` (marked inexact) for any projection with a
variable-width column, even when every projected column's `byte_size` is
known. A two-column `orders` scan reported 243.8 MB instead of 40.5 MB.
Summing the projected columns' sizes, as `Statistics::with_fetch`
already does, fixes the inflation but not item 1.

I also tried sizing the scan's partitions from the uncompressed size (a
fixed 8 or 32 MiB per partition): join modes don't change, since
`JoinSelection` doesn't look at partition counts, and TPC-DS got slower
(+5.6% and +6.9% sum of medians against not reporting sizes).

## Correctness check on real data

I converted Parquet to Vortex with the preview edition and compared, per
top-level column: the nested footer value as written, the nested value
re-read from the file, the legacy `ArrayStats` value parsed from the raw
footer, and the size recomputed from the input arrays.

| File | Columns | Result |
|---|---|---|
| All 24 TPC-DS SF0.1 tables | 431 | all match |
| ClickBench `hits_0.parquet` (1M rows, ~1.05 GB uncompressed) | 105 |
all match |

As a hand check, `income_band` (20 rows × 3 `int32`) reports 240 bytes.

## Checks

- `cargo clippy -p vortex-layout -p vortex-file -p vortex-datafusion
--all-targets --all-features -- -D warnings`
- `cargo +nightly-2026-09-10 fmt -p vortex-layout -p vortex-file -p
vortex-datafusion`
- `cargo test -p vortex-layout --lib` (729), `cargo test -p vortex-file
--all-features` (190 + doctests), `cargo test -p vortex-datafusion` (784
+ 14)
- `slt::datafusion::tpch/datafusion/{tpch,parquet}.slt` and
`slt::datafusion::tpcds/datafusion/{tpcds,parquet}.slt` on SF0.1 data
pass with the plans from `develop`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01UeniBBEktquqUp8DYPNJyR

---------

Signed-off-by: Joe Isaacs <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants