Skip to content

Separate the read and write IDs of array plugins - #11057

Open
connortsui20 wants to merge 2 commits into
developfrom
ct/array-plugin-read-write-ids
Open

connortsui20 wants to merge 2 commits into
developfrom
ct/array-plugin-read-write-ids

Conversation

@connortsui20

@connortsui20 connortsui20 commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Tracking Issue: #10701

Bottom of the decimal Widen stack. #11002 builds on it. No release gate.

Summary

A plugin can need another encoding's wire form without taking over its reader. serialized_ids() currently serves both jobs, so a Widen writer cannot emit a narrow decimal node while preserving the decimal reader.

Changes

Reader registration now uses read_ids(), while serialization validates its output against write_ids(). A writing session does not need a reader for every ID it writes. Existing plugins keep their current IDs, and tests cover reader ownership and the two validation contracts. LayoutPlugin is unchanged.

API Changes

Replaces ArrayPlugin::serialized_ids() with required read_ids() and write_ids() methods, with no defaults. Read IDs stay ordered oldest to newest.

@codspeed

codspeed Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

2 warnings

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

⚠️ 3 benchmarks with a significant change ran in a different software environment

Their base and head results were measured with a different OS or toolchain, such as compiler, runtime or library versions, so the changes might come from this difference rather than from the code. Open an affected benchmark to see what changed.

See affected benchmarks

⚡ 4 improved benchmarks
❌ 1 regressed benchmark
✅ 2313 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ CPU Simulation take_filter_primitive_small_uncached_random_mask_random_indices[4096, 10] 20.7 µs 24.4 µs -15.08%
⚡ CPU Simulation filter_powerlaw_by_random[10000] 1,300.8 ns 358.1 ns ×3.6
⚡ CPU Simulation test_repeated_scan_filter[int] 2.4 ms 2.1 ms +13.04%
⚡ CPU Simulation test_scan_filter_to_arrow_array[int] 2.8 ms 2.5 ms +11.23%
⚡ CPU Simulation test_scan_filter[int] 2.9 ms 2.6 ms +11%

Tip

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


Comparing ct/array-plugin-read-write-ids (9556b96) with develop (493967c)

Open in CodSpeed

@connortsui20
connortsui20 marked this pull request as ready for review October 30, 2026 18:48
@connortsui20 connortsui20 added the changelog/break A breaking API change label Oct 30, 2026
@joseph-isaacs

Copy link
Copy Markdown
Contributor

What are examples of the difference?

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
connortsui20 force-pushed the ct/array-plugin-read-write-ids branch from 851cd14 to 9556b96 Compare October 30, 2026 19:28
@connortsui20

Copy link
Copy Markdown
Member Author

@joseph-isaacs look at the next PR #11002

This branch has not been deployed

No deployments
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.

2 participants