Type edition memberships with a component kind - #9340
Open
joseph-isaacs wants to merge 10 commits into
Open
Conversation
Edition inclusions named a bare encoding id, and the ids had to be globally unique across everything an edition could ever cover. That made "which registry does this id resolve against?" implicit, and blocked layouts or functions joining editions without an id-collision rule. Every membership now carries a ComponentKind, declarations name members as EditionMember::array(&"vortex.alp"), and resolution is per kind: the file writer asks for enabled_array_encoding_ids and never sees other kinds. Inclusions are keyed by (kind, id), so an array encoding and a layout may share an id. All first-party declarations are arrays, pinned by a test. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
components_in scanned every declaration and filtered on kind, so resolving the array encodings walked layout and function members too. Nest the registry as kind -> id -> inclusion instead: a per-kind lookup now visits only that kind's map, and duplicate detection is a lookup in it. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
The spec described editions purely as encoding sets, with the kind bolted on as an opening paragraph. Give it a "What an edition contains" section that states the array-only scope plainly, says why the kind is recorded, and refuses to promise kinds that have no declared members or enforcement yet; then align the writing, change, and registry sections with it. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Scalar and aggregate function kinds had no declared members and nothing enforcing them, so they were speculative API. Keep the two kinds an edition can plausibly carry next, and mention the others once, where the extension point is documented. Trim the surface added along the way: EditionMember::new, members_in, and EditionInclusion::from_member had a single caller each, and ComponentKind needed no as_str beyond its Display. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Editions typed their members but only arrays were enforced, so a layout or a zone-map aggregate outside the enabled editions was written regardless. Name the kinds after what they are — array, layout, aggregate — and give each a filter where that kind is written. Layouts are interned once, in the footer serializer, so a LayoutContext carrying the permitted ids rejects a layout there exactly as the array context rejects an encoding. Aggregates are written into zoned layouts, so LayoutWriterContext carries the permitted ids and the zoned strategy drops the rest before it computes them: a zone map is an optimization, and a reader that meets an unknown aggregate skips it, so dropping costs pruning rather than data. The filters take plain id sets, like the array context already did — the layout crate stays edition-free and vortex-file resolves each set from the session. A kind the enabled editions declare nothing of stays unrestricted: today no edition declares a layout or an aggregate, and arming an empty set would forbid every layout and strip every zone map. Declaring the first member of a kind arms it. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Merging this PR will degrade performance by 16.22%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | cold_misaligned[(64, 256)] |
4.4 ms | 5.2 ms | -16.22% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/vortex-encoding-types-3xf1ie (6c4e0a2) with develop (402a5ac)
Footnotes
-
89 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. ↩
joseph-isaacs
added
the
action/bench-all
github-actions
Bot
removed
the
action/bench-all
Contributor
Polar Signals Profiling ResultsLatest Run
Previous Runs (15)
Powered by Polar Signals Cloud |
A forbidden aggregate was dropped from the zone map, so a writer configured outside its editions produced a file that quietly pruned worse than asked for. Fail the write instead, like a forbidden array or layout. The check runs after the accumulator has dropped the aggregates a column's dtype cannot hold, so it fires only for aggregates the write would have recorded. Declare those aggregates in core2026.08.0: min, max, bounded_min, bounded_max, sum, nan_count and null_count are what the default writer records, and declaring only some of them would fail every default write. Spatial columns carry one more — GeometryAabb registers itself as a zone stat — so vortex-spatial declares a spatial family and initialize() enables it, since a session that computes that statistic and may not write it is worse than useless. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Contributor
Benchmarks: String Encoding 📖vortex / vortex-file-compressed / ms (0.996x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / % (1.000x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: PolarSignals Profiling 📖Vortex (geomean): 0.997x ➖ datafusion / vortex-file-compressed / ns (0.997x ➖, 0↑ 1↓)
No file size changes detected. |
Contributor
Benchmarks: FineWeb NVMe 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.020x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.001x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.985x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.859x ✅, 5↑ 0↓)
duckdb / vortex-compact / ns (1.087x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.018x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.004x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.011x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.982x ➖, 3↑ 1↓)
duckdb / vortex-file-compressed / ns (0.994x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.988x ➖, 1↑ 0↓)
duckdb / parquet / ns (1.002x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: Clickbench Sorted on NVME 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.918x ➖, 2↑ 1↓)
datafusion / vortex-compact / ns (1.038x ➖, 0↑ 2↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.028x ➖, 1↑ 1↓)
duckdb / vortex-compact / ns (0.987x ➖, 3↑ 2↓)
duckdb / parquet / ns (1.007x ➖, 0↑ 0↓)
File Size Changes (200 files changed, +0.0% overall, 113↑ 87↓)
Totals:
|
Contributor
Benchmarks: TPC-DS SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.993x ➖, 1↑ 3↓)
datafusion / vortex-compact / ns (0.995x ➖, 3↑ 1↓)
datafusion / parquet / ns (0.988x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.021x ➖, 4↑ 10↓)
duckdb / vortex-compact / ns (0.995x ➖, 1↑ 1↓)
duckdb / parquet / ns (0.992x ➖, 3↑ 1↓)
No file size changes detected. |
joseph-isaacs
added
the
action/bench-all
Claude
github-actions
Bot
removed
the
action/bench-all
joseph-isaacs
added
changelog/feature
Claude
github-actions
Bot
removed
the
action/bench-all
Contributor
Benchmarks: Random Access 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
random-access / vortex-file-compressed / ns (0.998x ➖, 0↑ 0↓)
random-access / parquet / ns (1.000x ➖, 0↑ 0↓)
random-access / lance / ns (1.012x ➖, 0↑ 1↓)
|
Contributor
Benchmarks: FineWeb S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.339x ❌, 0↑ 4↓)
datafusion / vortex-compact / ns (1.095x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.025x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.049x ➖, 0↑ 1↓)
duckdb / parquet / ns (0.831x ➖, 1↑ 0↓)
|
Contributor
Benchmarks: Statistical and Population Genetics 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
duckdb / vortex-file-compressed / ns (1.016x ➖, 0↑ 1↓)
duckdb / vortex-compact / ns (0.978x ➖, 1↑ 1↓)
duckdb / parquet / ns (1.008x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: Clickbench on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.979x ➖, 2↑ 0↓)
datafusion / vortex-compact / ns (1.002x ➖, 1↑ 1↓)
datafusion / parquet / ns (1.014x ➖, 1↑ 1↓)
duckdb / vortex-file-compressed / ns (0.969x ➖, 3↑ 1↓)
duckdb / vortex-compact / ns (0.987x ➖, 6↑ 4↓)
duckdb / parquet / ns (1.003x ➖, 1↑ 1↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=10 on S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-compact / ns (1.048x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.104x ➖, 0↑ 5↓)
duckdb / vortex-compact / ns (1.025x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.080x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: Appian on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-compact / ns (0.999x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.994x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.998x ➖, 0↑ 0↓)
File Size Changes (10 files changed, -63.8% overall, 0↑ 10↓)
Totals:
|
Contributor
Benchmarks: TPC-H SF=10 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.999x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.011x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.007x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.014x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.981x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.004x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on S3 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.076x ➖, 0↑ 2↓)
datafusion / vortex-compact / ns (0.991x ➖, 2↑ 3↓)
datafusion / parquet / ns (0.998x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.075x ➖, 0↑ 2↓)
duckdb / vortex-compact / ns (1.034x ➖, 1↑ 1↓)
duckdb / parquet / ns (1.034x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: Compression 📖vortex / vortex-file-compressed / ns (0.996x ➖, 1↑ 0↓)
vortex / vortex-file-compressed / bytes (1.001x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / ratio (1.000x ➖, 1↑ 0↓)
vortex / parquet / ns (0.996x ➖, 0↑ 0↓)
vortex / parquet / bytes (1.000x ➖, 0↑ 0↓)
|
joseph-isaacs
marked this pull request as ready for review
August 11, 2026 14:03
joseph-isaacs
force-pushed
the
claude/vortex-encoding-types-3xf1ie
branch
from
August 11, 2026 14:18
a7ecb53 to
953e03c
Compare
The editions module docs merge with the doc comment on `pub mod editions;` in lib.rs, where the link resolves against the crate root and there is no `ComponentKind` in scope. Qualify it like its neighbours in the same block. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Zone maps exist to prune, and a zone sum prunes nothing: `ZoneMap::prune` lowers predicates, and no predicate is answered by a per-zone total. The aggregate was recorded because it always had been, and it then had to be declared in `core2026.08.0` for writes to keep succeeding once the aggregate filter was armed - a hostage situation, not a justification. Its semantics are also unsettled: null-on-empty was changed in #9113 and reverted in #9324 within the same week. That is not a stat to freeze into an edition whose members are permanent. Drop `Sum` from the default zoned aggregates and `vortex.sum` from `core2026.08.0`, so the writer and the declaration stay consistent. File-level statistics still record `Stat::Sum` via `PRUNING_STATS`; that mechanism is separate and unaffected, as is the legacy-read path in `zoned/schema.rs` that still understands a sum in older files. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
joseph-isaacs
force-pushed
the
claude/vortex-encoding-types-3xf1ie
branch
from
August 11, 2026 14:20
953e03c to
4166905
Compare
Not writing a sum per zone makes every file with a zone map smaller. The two `os.path.getsize` doctests in `VortexWriteOptions.compact` pin exact byte counts, so they move with it: 215932 -> 215716 and 55060 -> 54920. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
joseph-isaacs
force-pushed
the
claude/vortex-encoding-types-3xf1ie
branch
from
August 11, 2026 14:30
bf4c745 to
2fe3c66
Compare
The branch was 20 commits behind, including the Rust toolchain bump from 1.91.0 to 1.97.1 (#9333). CodSpeed builds the base from develop and the head from this branch, so the two were compiled by toolchains six releases apart, which is what the `cold_misaligned` instruction-count difference measured. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
joseph-isaacs
force-pushed
the
claude/vortex-encoding-types-3xf1ie
branch
from
August 11, 2026 15:01
836d2f2 to
6c4e0a2
Compare
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
1 participant
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.
Add support for edition object other than array encodings