Skip to content

Select Decimal compression format from writer permissions - #9977

Closed
mhk197 wants to merge 8 commits into
developfrom
mk/configurable-schemes
Closed

mhk197 wants to merge 8 commits into
developfrom
mk/configurable-schemes

Conversation

@mhk197

@mhk197 mhk197 commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Enable compression of wide decimals when the writer permits vortex.decimal_byte_parts.v2. Writers restricted to v1 continue to leave wide decimals canonical.

Changes

  • Keep Decimal v1 in all_schemes() and store static scheme references plus an optional allowlist on the builder.
  • Store permissions with with_allowed_encodings. Repeated calls intersect; during build(), select the latest permitted Decimal version and filter unsupported schemes, including schemes registered after permissions were supplied. Without an allowlist, registered configurations are preserved.
  • Compress the most significant and lower parts of wide decimals independently. Single-part decimals continue to serialize as v1.
  • Apply the CUDA preset after supplying permissions so it removes Decimal v2 from the stored allowlist. Later intersections cannot re-enable v2.
  • Migrate the writer, bindings, CLI, CUDA integration, and benchmarks to the deferred configuration API.
  • Cover format selection, explicit registrations, intersecting restrictions, CUDA compatibility, child compression, and edition-aware file round trips.

API Changes

ALL_SCHEMES is replaced by all_schemes(). DecimalScheme::new(v2) replaces construction as a unit struct. retain_allowed_encodings is replaced by with_allowed_encodings, which applies permissions at build time and can change explicitly registered Decimal versions. Static scheme registration is preserved.

Supply the initial allowlist before applying the CUDA preset:

BtrBlocksCompressorBuilder::default()
    .with_allowed_encodings(&allowed_serialized_ids)
    .only_cuda_compatible()
    .build()

Validation

Reviewed the source diff. Tests, builds, linting, and formatting were not run, per repository instructions.

@mhk197 mhk197 added the changelog/break A breaking API change label Sep 22, 2026
Use shared scheme instances and deferred constructors so each compressor
receives its final serialized ID restrictions. Keep scheme availability
gating separate from construction through produced_encodings.

Configure one DecimalScheme to retain v1 output for narrow values and
enable v2 lower parts only when the serialized ID is permitted.

Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>
Remove the single-field SchemeConfig wrapper and pass the allowed IDs to
scheme constructors directly. Keep availability gating separate from
DecimalScheme construction.

Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>
Replace produced_encodings with a boolean predicate over allowed serialized
IDs. Check each configured instance's direct outputs without allocating an
encoding list, while preserving independent scheme construction.

Require both Decimal wire IDs when v2 is enabled and cover rejection of a
preconfigured v2 scheme by a v1-only writer.

Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>
…bility

Make scheme constructors return SchemeRef consistently, allowing the
builder to reuse the default factory catalog and existing instances.
Apply exclusions only to registrations present when they were requested,
so callers can replace configured defaults.

Keep existing Decimal registrations in place while constraining the CUDA
preset to v1 output. Add regression coverage for all three review findings.

Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>
Use explicit scheme IDs for deferred constructors, restoring immediate
exclusions and duplicate checks without probing factories. Restore
produced_encodings and centralize serialized-ID availability checks.

Default Decimal to v1 unless the supplied serialized IDs permit v2.
Persist the CUDA preset's v2 restriction across later registrations and
reject preconfigured schemes that cannot honor it. Cover writer edition
selection, custom strategies, and non-constant decimal parts in tests.

Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>
Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>
@mhk197
mhk197 force-pushed the mk/configurable-schemes branch from 4ccac0f to 7c1d927 Compare September 22, 2026 15:52
Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>
@codspeed

codspeed Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 17.32%

⚠️ 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.

❌ 2 regressed benchmarks
✅ 196 untouched benchmarks
⏩ 2303 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ scalar_subtract_neon 12 µs 15.1 µs -20.66%
❌ filtered_sink_i64_avx512[OneNullInEight] 22.8 µs 26.5 µs -13.83%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing mk/configurable-schemes (195c7ae) with develop (27a2f10)

Open in CodSpeed

Footnotes

  1. 2303 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. ↩

Signed-off-by: "Matt Katz" <mhkatz97@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/break A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant