Repository navigation
Add uncompressed_size_in_bytes to the preview edition - #10922
Merged
Merged
Conversation
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
Merging this PR will improve performance by 30.79%2 warnings
Performance Changes
Tip Curious why performance improved? Comment Comparing Footnotes |
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
Signed-off-by: Claude <[email protected]>
myrrc
reviewed
Oct 28, 2026
myrrc
approved these changes
Oct 28, 2026
joseph-isaacs
changed the base branch from
develop
to
ji/uncompressed-size-fixes
October 28, 2026 13:35
Signed-off-by: Claude <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01UeniBAJcn1sFtocvffo36c
Signed-off-by: Claude <[email protected]>
Signed-off-by: Claude <[email protected]>
…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
joseph-isaacs
enabled auto-merge (squash)
October 29, 2026 15:41
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]>
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.
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_bytesis in no edition, so the footer can't record a field's uncompressed size. This PR adds it to the draftpreview2026.08.0edition, so writers can opt in by enabling preview. Frozen core editions are unchanged.Changes
vortex-edition:preview2026.08.0addsEditionMember::aggregate("vortex.uncompressed_size_in_bytes").vortex/editions/preview/preview2026.08.0.toml: regenerated withcargo 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