Filter compression schemes by serialized IDs and upgrade decimal modes by mhk197 · Pull Request #9992 · vortex-data/vortex

mhk197 · 2026-09-22T18:55:30Z

Summary

Configure BtrBlocks compression schemes from the serialized array IDs the writer is allowed to emit. The builder uses those permissions to select compatible scheme versions and filter unsupported schemes once, when build() is called.

The two main changes are defaulting to the default core edition's encodings and adding Scheme::try_upgrade for optional upgrades such as decimal v1 to v2.

Defaults use the default core edition

BtrBlocksCompressorBuilder::default() now takes its permitted serialized IDs from DEFAULT_CORE_EDITION, including encodings inherited from earlier core editions. BtrBlocksCompressor::default() inherits this behavior.

The current default core edition permits decimal v1, so these defaults keep decimal compression in v1 mode. A caller that creates a default write strategy just to customize layout therefore retains decimal output compatible with the default edition.

DEFAULT_CORE_EDITION must live in vortex-edition to avoid a dependency cycle. The new array_ids_for_edition() helper resolves static edition membership in that crate, keeping edition traversal out of the compressor builder.

Scheme upgrades with `try_upgrade`

Scheme::produced_encodings() remains the required declaration of the serialized IDs a scheme can emit. The new optional hook lets a scheme offer a newer configuration when the supplied permissions support it:

fn try_upgrade(&self, allowed_serialized_ids: &AllowedSerializedIds) -> Option<&dyn Scheme>

The default implementation returns None. During build(), the builder makes one ordered pass over all registered schemes:

Call try_upgrade with the permitted serialized IDs.
Use the returned scheme, or keep the original scheme if the result is None.
Keep the selected scheme only if every ID in its produced_encodings() is permitted.

Returning None means there is no upgrade; the original scheme still goes through filtering. An upgrade preserves the scheme's SchemeId and must not downgrade its configuration. Selection order is preserved, and the same rules apply to schemes added through with_new_scheme().

Only the decimal scheme overrides this hook. Other schemes continue to declare their outputs and use the default implementation.

Decimal v1 and v2

DecimalScheme::default() and the default scheme list use v1. The upgrade hook returns a static v2 scheme when both decimal v1 and v2 serialized IDs are permitted.

V1: narrows decimal values and leaves values still wider than i64 in canonical form.
V2: can split wide i128 and i256 values into a signed most significant part and unsigned lower parts, compressing each part independently.

V2 requires both IDs because single-part arrays still serialize as v1. An explicitly registered v2 scheme is filtered out if either ID is missing; it is never downgraded to v1.

only_cuda_compatible() removes the v2 ID from the permitted set because CUDA does not support lower decimal parts. This prevents v1-to-v2 upgrades and filters explicit v2 schemes, including those registered after applying the preset. Apply the preset after permission changes, since set_allowed_encodings and allow_encodings can re-enable v2.

Builder and writer integration

Permissions can be supplied at construction or updated before build(), replacing retain_allowed_encodings(). AllowedSerializedIds is a HashSet<ArrayId> alias:

new(ids) starts with the default schemes and the supplied permissions.
empty() starts with no schemes and no permitted serialized IDs.
default() starts with the default schemes and the default core edition's permissions.
set_allowed_encodings(ids) replaces the permitted IDs.
allow_encodings(ids) adds permitted IDs while preserving the existing set.

Both permission methods accept an iterator of serialized IDs and return the builder for chaining. Upgrades and filtering use the final permissions when build() is called.

Schemes whose outputs are not permitted are silently omitted at build time. To use an encoding outside the default core edition, supply its serialized ID through new(ids) or allow_encodings(ids) as well as registering its scheme. For example, start from empty().allow_encodings(ids) when configuring schemes individually.

When no explicit write strategy is supplied, the file writer constructs the compressor using registered serialized IDs permitted by the session's enabled editions. With edition enforcement disabled, it uses all registered IDs, which can enable decimal v2. Explicit write strategies retain their supplied configuration; callers targeting older or custom editions must supply matching permissions.

Resolve allowed and forbidden serialized IDs when building the compressor, select compatible Decimal modes, and compress wide decimal parts independently. Default to Decimal v2 while keeping the CUDA preset restricted to v1. Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>

Replace produced_encodings with Scheme::configure and store owned AllowedSerializedIds in the BtrBlocks builder. Configure schemes in one pass, select decimal v2 whenever permitted, and preserve CUDA restrictions. Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>

Restore produced_encodings and add an optional try_upgrade hook. Default decimals to v1, upgrade only with an explicit allowlist, and keep CUDA exclusions effective. Use static decimal instances and cover default writer compatibility. Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>

codspeed · 2026-09-22T21:19:45Z

Merging this PR will improve performance by 55.09%

⚠️

Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️

3 benchmarks measured no execution time

Nothing ran under measurement, usually because the compiler removed the code under test. These results are not comparable, so they count as unchanged.

Preventing compiler optimizations

⚠️

Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 4 improved benchmarks
✅ 2258 untouched benchmarks
⏩ 287 skipped benchmarks¹
🗄️ 1 archived benchmark run²

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
⚡	Simulation	`take_fsl_u32_random[128, 100]`	137.9 µs	77.1 µs	+78.76%
⚡	Simulation	`take_fsl_u64_random[64, 10]`	54 µs	30.9 µs	+74.86%
⚡	Simulation	`take_fsl_f16_random[256, 100]`	141.7 µs	88.2 µs	+60.72%
⚡	Simulation	`take_fsl_random[64, 100]`	144.5 µs	125.4 µs	+15.17%
⚠️	Simulation	`fixed_16_advancing_ptr_safe[100]`	< 1 ns	< 1 ns	N/A
⚠️	Simulation	`preverify_advancing_ptr_unchecked[1000]`	< 1 ns	< 1 ns	N/A
⚠️	Simulation	`preverify_advancing_ptr_unchecked[10000]`	< 1 ns	< 1 ns	N/A

Tip

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

_{Comparing mk/filter-schemes-on-serialized-ids (0c415a7) with develop (1eb5b43)}

287 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. ↩
1 benchmark was run, but is now archived. If it was deleted in another branch, consider rebasing to remove it from the report. Instead if it was added back, click here to restore it. ↩

Accept serialized IDs when constructing BtrBlocks builders and derive default permissions from the default core edition. Replace permission variants with a set, preserve CUDA exclusions, and migrate writer and benchmark callers to constructor-based permissions. Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>

Resolve static edition array IDs in vortex-edition and document how builder permissions filter schemes registered later. Explicitly permit Delta in the scheme selection and trace tests while preserving their assertions. Fix the decimal upgrade lint, redundant qualifications, and formatting. Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>

mhk197 · 2026-09-23T01:55:08Z

 use vortex_session::VortexSession;

-/// The `core` edition enabled for writing by the default Vortex session.
-pub const DEFAULT_CORE_EDITION: EditionId = CORE_2026_08_3;


This should be kept here but vortex depends on btrblocks, so import cycle

Make empty() start without registered schemes or serialized ID permissions. Allow callers to set or extend permissions before build, while keeping the default core edition permissions in default(). Update callers and cover final permission filtering, decimal upgrades, and CUDA preset ordering. Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>

joseph-isaacs · 2026-09-23T08:06:37Z

    &binary::VarBinScheme,
    // Decimal schemes.
-    &decimal::DecimalScheme,
+    // Use v1 by default and let builder upgrade to v2 if permitted by edition.


if permitted by the allow encodings (editions are not a framework concern)

joseph-isaacs · 2026-09-23T08:08:49Z

+    /// `None` keeps the original scheme. An upgrade must preserve the [`SchemeId`] and must not
+    /// downgrade the registered configuration. The caller checks the selected scheme's
+    /// [`produced_encodings`](Self::produced_encodings) before using it.
+    fn try_upgrade(&self, _allowed_serialized_ids: &AllowedSerializedIds) -> Option<&dyn Scheme> {


why can we not check this on construction of the compressor

joseph-isaacs · 2026-09-23T08:10:23Z

 vortex-compressor = { workspace = true }
 vortex-datetime-parts = { workspace = true }
 vortex-decimal-byte-parts = { workspace = true }
+vortex-edition = { workspace = true }


Can we avoid this? Ideally you pass id on construction.

Compressor shouldn't need to know about edition just allowed ids

I think this is because of Default impl. I think you can construct the compressor and then only filter it in the write strategy

Yeah this is because of Default, which I thought we didn't want to completely break.

In order to avoid this we either need to:

Remove Default and have compressor constructors always take a permitted id list.

No ID is permitted to default compressor, and you set them later

A subset of the IDs in ALL_SCHEMES are permitted in default compressor via a default allowlist.

I prefer 1 because 2 is effectively a hard break and 3 requires more bookkeeping

I think you can have unfiltered list here that needs to have ids supplied during construction?

IE we postfilter

that would not solve the delta problem we had though iiuc

Delta is not in any edition so it would get removed from the supported strategies

connortsui20 · 2026-09-23T13:36:51Z

it might be good to split out this change into a pr stack

connortsui20 · 2026-09-23T13:39:28Z

 /// This list is order-sensitive: the builder preserves this order when constructing
 /// the final scheme list, so that tie-breaking is deterministic.
+///
+/// If a scheme can be configured to support different editions like


I think in general (like what Joe said below) we want to be very, very intentional with our wording wrt editions, versions, allowed wire IDs, etc, because this can easily become confusing and counterintuitive. That was one of the things I was trying to do on the versioning docs (still a PR, probably can still be improved)

connortsui20 · 2026-09-23T13:41:12Z

+    /// An empty set permits no serialized IDs. Upgrades and filtering are deferred until
+    /// [`Self::build`], including for schemes registered later. Schemes are never downgraded.


yeah I think it would be good to merge the versioning PR and then have comments that reference that because in a vacuum this can be very confusing

connortsui20 · 2026-09-23T13:42:26Z

+/// The serialized IDs permitted for compression schemes.
+pub type AllowedSerializedIds = HashSet<ArrayId>;


shouldn't this be opaque? so that we can have both the allow and forbidden list in here?

Since we always supply ids now, we just need the allowlist

I generally like types being opaque (so you can add methods to it) but I'll let you decide whats best

connortsui20 · 2026-09-23T13:45:10Z

+    /// Uses the default core edition's serialized array IDs. Use [`Self::allow_encodings`] to
+    /// permit additional encodings; otherwise, schemes requiring them are omitted at build.
    fn default() -> Self {
+        Self::new(array_ids_for_edition(&DEFAULT_CORE_EDITION).collect())
+    }
+}


I think it should be fine to break this (remove it) so that vortex-btrblocks doesn't have to depend on vortex-edition

mhk197 added the changelog/break A breaking API change label Sep 22, 2026

mhk197 changed the title ~~Configure compressor schemes from serialized ID permissions~~ configure scheme with mode Sep 22, 2026

connortsui20 self-requested a review September 22, 2026 18:59

mhk197 added 2 commits September 22, 2026 16:38

mhk197 changed the title ~~configure scheme with mode~~ Filter compression schemes by serialized IDs and upgrade decimal modes Sep 22, 2026

mhk197 added 2 commits September 22, 2026 21:11

mhk197 commented Sep 23, 2026

View reviewed changes

mhk197 requested review from joseph-isaacs and robert3005 September 23, 2026 02:58

joseph-isaacs reviewed Sep 23, 2026

View reviewed changes

connortsui20 reviewed Sep 23, 2026

View reviewed changes

Comment thread vortex-btrblocks/src/schemes/decimal.rs

connortsui20 Sep 23, 2026

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might be good to split out this change into a pr stack

connortsui20 reviewed Sep 23, 2026

View reviewed changes

mhk197 closed this Sep 24, 2026

		/// An empty set permits no serialized IDs. Upgrades and filtering are deferred until
		/// [`Self::build`], including for schemes registered later. Schemes are never downgraded.

		/// The serialized IDs permitted for compression schemes.
		pub type AllowedSerializedIds = HashSet<ArrayId>;

Conversation

mhk197 commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Defaults use the default core edition

Scheme upgrades with try_upgrade

Decimal v1 and v2

Builder and writer integration

Uh oh!

codspeed Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will improve performance by 55.09%

Performance Changes

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mhk197 commented Sep 22, 2026 •

edited

Loading

Scheme upgrades with `try_upgrade`

codspeed Bot commented Sep 22, 2026 •

edited

Loading