Skip to content

Move array serde from VTable to explicit ArrayPlugin impls - #11285

Merged
mhk197 merged 8 commits into
developfrom
mk/explicit-array-plugins
Nov 6, 2026
Merged

mhk197 merged 8 commits into
developfrom
mk/explicit-array-plugins

Conversation

@mhk197

@mhk197 mhk197 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Removes the blanket impl<V: VTable> ArrayPlugin for V and the serialize / deserialize methods on VTable. Every serializable encoding now implements ArrayPlugin explicitly on its vtable type, in a plugin.rs child module next to its impl VTable. Redo of #10111 on current develop.

Changes

  • VTable covers only in-memory behaviour; serde is impl ArrayPlugin for X.
  • Serde bodies are ported verbatim, so wire formats are unchanged.
  • read_ids / write_ids default to vec![self.id()], what the blanket impl declared. Plugins with one wire ID no longer spell them out, including ALPPatchedPlugin, ScalarFnArrayPlugin and the Python stub.
  • Inside an impl the wire ID is ArrayPlugin::id(self); only the id method itself delegates to VTable::id.
  • session.arrays().register(X) is unchanged for every encoding that was serializable.
  • In-memory-only vtables lose the two stub methods and get no plugin: Filter, Slice, Interleave, ScalarFn, ScalarFnValidity, ForeignArray, PiecewiseSequence, Widen, Shared, test-harness vtables.
  • DecimalPlugin (Read legacy vortex.decimal nodes into the canonical array or a Widen #10995) becomes impl ArrayPlugin for Decimal: one wire ID, same legacy-node reads, no separate type. Its module, StoredDecimal and tests stay where they are.
  • Multi-ID plugins (BitPackedPlugin, FoRPlugin, DecimalBytePartsPlugin) and WidenPlugin are untouched; ALPPatchedPlugin now delegates to ArrayPlugin::deserialize(&ALP, ..).
  • PythonVTable keeps a stub plugin so the Python register API still compiles.
  • Vtable files that gained a plugin.rs child are directory modules (array.rs becomes array/mod.rs, 14 pure renames). ALP's own impl and ALPPatchedPlugin share alp/array/plugin.rs.
  • Tests that called the vtable serde directly (ALP, ALPRD, Dict, List, RunEnd, VarBin, RLE, serde.rs, unary, zstd buffers) go through ArrayPlugin instead.
count
new impl ArrayPlugin (vortex-array) 20
new impl ArrayPlugin (encodings + python) 17
vtables that only lose the stub methods 14

API changes

  • VTable loses serialize and deserialize. An external vtable that was registered through the blanket impl must now impl ArrayPlugin to be registrable.
  • ArrayPlugin::read_ids / write_ids gain defaults of vec![self.id()].
  • vortex_array::arrays::DecimalPlugin is removed; register Decimal instead.

Behaviour changes

  • PiecewiseSequence is no longer registered by ArraySession::default(); it never serialized, and its error is now "is not registered for serialization".
  • The writer seeds the footer's encoding list from the session registry, so dropping vortex.piecewise-sequence makes every default-session file 48 bytes smaller. test_stock_ticker_file_size expectations move accordingly (70060 → 70012, 69996 → 69948, 55144 → 55096).
  • DecimalByteParts, BitPacked and FoR can no longer be registered bare, so bare_vtable_refuses_serde and vtable_serde_requires_plugin are removed.

Testing

  • cargo clippy --all-targets --all-features -- -D warnings and cargo +nightly-2026-09-10 fmt --check on every touched crate (vortex-array, vortex-file, vortex-layout, vortex-python and the 13 encodings crates).
  • cargo nextest run on all touched crates plus vortex-ipc and vortex-scan: 8981 passed, 0 failed; cargo test --doc on vortex-array, fastlanes, decimal-byte-parts and alp.
  • cargo nextest run -p vortex-file -E 'test(test_stock_ticker_file_size)' with and without --features zstd, to derive the new sizes.

Follow-up

  • Pre-existing, unchanged here: the Patched, RLE, OnPair, ALPRD, FSST and Zstd deserializers index children or buffers before checking their counts, so a malformed node panics in SerializedArray::child instead of returning an error. Worth hoisting the count checks in a separate PR.
  • Fold FoRPlugin / BitPackedPlugin / DecimalBytePartsPlugin into impl ArrayPlugin for FoR etc. if wanted.
  • Overlaps with Move array serde off VTable onto explicit ArrayPlugin impls #11298, which makes the same change with inline impls and trait defaults for read_ids / write_ids.

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197
mhk197 force-pushed the mk/explicit-array-plugins branch from 8c0c881 to a18ed1b Compare November 6, 2026 15:30
@mhk197 mhk197 added the changelog/break A breaking API change label Nov 6, 2026
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197
mhk197 marked this pull request as ready for review November 6, 2026 15:57

@robert3005 robert3005 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I shall trust that this is just a move not a semantic change

@codspeed

codspeed Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

1 warning

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

✅ 1917 untouched benchmarks
⏩ 47 skipped benchmarks1


Comparing mk/explicit-array-plugins (503854f) with develop (7854a19)

Open in CodSpeed

Footnotes

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

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.

Wait I thought we were going to change this back to a normal Decimal instead of DecimalPlugin

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Switching

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197
mhk197 force-pushed the mk/explicit-array-plugins branch from 129204e to 157921a Compare November 6, 2026 16:12
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
pub struct DecimalPlugin;

impl ArrayPlugin for DecimalPlugin {
impl ArrayPlugin for Decimal {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@connortsui20 decimal change here

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

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197
mhk197 force-pushed the mk/explicit-array-plugins branch from e9cd2a5 to 503854f Compare November 6, 2026 17:38

@connortsui20 connortsui20 left a comment

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.

ship it

@mhk197
mhk197 enabled auto-merge (squash) November 6, 2026 17:48
@mhk197
mhk197 merged commit 47daf16 into develop Nov 6, 2026
76 checks passed
@mhk197
mhk197 deleted the mk/explicit-array-plugins branch November 6, 2026 17:49

This branch was successfully deployed

1 active deployment
vercel-docs — 503854f6 Deployed Nov 6, 2026 by mhk197 via Publish preview / Deploy #8864
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.

3 participants