Skip to content

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

Merged
connortsui20 merged 2 commits into
developfrom
ct/array-plugin-read-write-ids
Nov 2, 2026
Merged

Separate the read and write IDs of array plugins#11057
connortsui20 merged 2 commits into
developfrom
ct/array-plugin-read-write-ids

Conversation

@connortsui20

@connortsui20 connortsui20 commented Oct 30, 2026 •
edited
Loading

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.

@connortsui20
connortsui20 added this pull request to stack #11058 October 30, 2026 18:03
This was referenced Oct 30, 2026
Closed
Merged
@codspeed

codspeed Bot commented Oct 30, 2026 •
edited
Loading

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

joseph-isaacs commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

What are examples of the difference?

@connortsui20
connortsui20 force-pushed the ct/array-plugin-read-write-ids branch from 851cd14 to 9556b96 Compare October 30, 2026 19:28
@connortsui20

connortsui20 commented Oct 30, 2026

Copy link
Copy Markdown
Member Author

@joseph-isaacs look at the next PR #11002

@connortsui20
connortsui20 merged commit ea94807 into develop Nov 2, 2026
78 of 80 checks passed
@connortsui20
connortsui20 deleted the ct/array-plugin-read-write-ids branch November 2, 2026 13:50
This was referenced Nov 2, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robert3005 robert3005 robert3005 approved these changes
@mhk197 mhk197 Awaiting requested review from mhk197

Assignees

No one assigned

Labels

changelog/break A breaking API change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@connortsui20 @joseph-isaacs @robert3005