You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[EXPERIMENT] Narrow internal children with encode_values - #10943
Internal integer children now keep their signedness when the compressor narrows them. Each call to the deprecated encode_values marks a child whose dtype policy still needs a decision.
Changes
Replace all 24 narrowing calls across 17 files with WidenArray::encode_values, using the same TODO(connor) reason at each call.
Preserve signed List offsets, signed ListView offsets and sizes, FSST lengths and code offsets, and DateTimeParts parts. Non-negative signed values from 128 to 255 now need i16 rather than u8. Unsigned children keep their behavior.
Port the boundary, nullability, and empty-input cases to the existing encode_values tests. Add compact list and string serialization round trips that exclude vortex.widen. The list case asserts signed offsets and fails with the old narrowing.
Regenerate five golden snapshots. Only signed dtype and scalar labels change. Every byte count stays the same.
API Changes
PrimitiveArrayExt::narrow is removed. Newly written files retain signed dtypes for signed internal children. Encoding IDs and metadata formats do not change.
Source audit and signed-child readers
The brief's table contains 24 calls, rather than 25. All 24 calls are migrated. The binary VarBin scheme builds u64 offsets, and OnPair produces unsigned children, so those sites do not change signedness. Dictionary codes, RunEnd ends, RLE indices and offsets, and patch and sparse indices are also unsigned. List and ListView callers can provide signed children. FSST produces i32 lengths and i32 or i64 offsets. DateTimeParts produces signed parts.
All locations below are relative to the PR source tree. CUDA results are static inspection only.
Dispatches by unsigned width and reads the identical representation of non-negative signed offsets.
FSST and OnPair lengths
CUDA Arrow offsets
vortex-cuda/kernels/src/arrow_offsets.cu:56
Kernels include all eight integer types.
OnPair code offsets
CUDA window boundaries
vortex-cuda/kernels/src/onpair.cu:107
Kernels include all signed and unsigned offset widths. Token codes remain unsigned.
Snapshots
Snapshot
Change
Root bytes, unchanged
compact/list_of_int_runs
Offsets u16 to i16.
4,434
regular/list_of_int_runs
Offsets and their patch values u16 to i16.
11,146
regular/string_fsst_structured
Lengths and scalar labels u8 to i8, code offsets u32 to i32.
151,313
regular/string_low_cardinality
Dictionary values' FSST lengths and offsets u8 to i8.
8,373
regular/struct_mixed
Category dictionary values' FSST lengths and offsets u8 to i8.
57,525
Every stored range fits the signed type at the existing width. No byte-count changes need explanation.
Checks and compatibility
Scoped clippy for vortex-array, vortex-btrblocks, vortex-compressor, and vortex-fastlanes, with all targets, all features, and warnings denied: passed.
Formatting with nightly-2026-09-10 for those four crates: passed.
cargo nextest run -p vortex-array: 5,189 passed, one skipped.
cargo nextest run -p vortex-btrblocks --all-features: 92 passed.
cargo nextest run -p vortex-compressor -p vortex-fastlanes: 1,240 passed.
cargo nextest run -p vortex-fsst -p vortex-onpair -p vortex-datetime-parts: 242 passed, one skipped.
cargo nextest run -p vortex-file: 189 passed. No file-test changes were needed.
All three golden variants passed during snapshot regeneration.
Before migration, the new list regression failed with actual U16 and expected I16. After migration, both cases passed.
No .narrow( calls remain. All 24 migrated calls have the exact deprecation reason. git diff --check passed.
Commit 1 passed scoped clippy and all 5,209 array tests before the call sites changed. Commit hooks passed for both commits without bypasses. The pre-push hook also passed workspace-wide clippy with all targets, all features, and warnings denied.
Old-reader check: the directly built 0.90.0 reader passed 36 fixtures, failed none, and skipped the same seven newer decimal fixtures for both base 9bf403a1ce and WD6. The per-fixture results match exactly in subset mode. All 43 fixtures were generated on each revision. The ClickBench download succeeded, and both runs reused the same deterministic sample. Both temporary compatibility worktrees were removed.
Coverage gap: compressed fixtures are TPC-H lineitem, TPC-H orders, and ClickBench, with no list fields. Synthetic List and ListView fixtures use flat layouts. No new fixture was added. The new integration test covers compressed lists with the current reader.
CUDA cannot be built locally. The PR path filter skipped the dedicated CUDA workflow, so a manual run was started on exact head b2bd758df8. Its results are pending.
Performance evidence and remaining CI work
Local btrblocks comparisons were skipped at the maintainer's request. CodSpeed does not run the btrblocks benchmarks.
The valid WD3 report compares 7dd6ba4 with 2fb259b. All 12 take_map cases are classified NoChange, with latency differences from -1.03% to +0.96%. The valid WD4 report compares 419b196 with 37031b1. All 12 cases are NoChange, with latency differences from -0.31% to +6.34%. WD4 carries a runner-difference warning. Neither report has an unexpected-base condition. WD1's comment uses head 75c7f4a, while its final head is a56a48e, so it is excluded as evidence for the final PR.
The take_map fixture uses u64 indices and values directly and calls no narrowing code. The selected hashbrown 0.17.1 default builder uses foldhash 0.2.0 random state, with per-hasher and shared seeds. A fixed fixture RNG does not fix the hash layout. This establishes a possible source of variance, not the cause of the old regressions.
WD6's own CodSpeed comparison, any needed flame graph analysis, and develop-history comparison remain pending the PR benchmark run. Only reports with the correct base and head will be used. No take_map code changed.
System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.
Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.
30 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them. ↩
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
connortsui20
changed the title
Summary
Widenencoding for integers and decimals #10733Internal integer children now keep their signedness when the compressor narrows them. Each call to the deprecated
encode_valuesmarks a child whose dtype policy still needs a decision.Changes
WidenArray::encode_values, using the sameTODO(connor)reason at each call.i16rather thanu8. Unsigned children keep their behavior.encode_valuestests. Add compact list and string serialization round trips that excludevortex.widen. The list case asserts signed offsets and fails with the old narrowing.API Changes
PrimitiveArrayExt::narrowis removed. Newly written files retain signed dtypes for signed internal children. Encoding IDs and metadata formats do not change.Source audit and signed-child readers
The brief's table contains 24 calls, rather than 25. All 24 calls are migrated. The binary VarBin scheme builds
u64offsets, and OnPair produces unsigned children, so those sites do not change signedness. Dictionary codes, RunEnd ends, RLE indices and offsets, and patch and sparse indices are also unsigned. List and ListView callers can provide signed children. FSST producesi32lengths andi32ori64offsets. DateTimeParts produces signed parts.All locations below are relative to the PR source tree. CUDA results are static inspection only.
vortex-array/src/arrays/list/array.rs:213,:231,:308vortex-array/src/arrays/listview/array.rs:239,:269,:435;vortex-array/src/arrays/listview/rebuild.rs:165vortex-array/src/arrays/varbin/array.rs:238;vortex-array/src/arrays/varbin/vtable/canonical.rs:29encodings/fsst/src/canonical.rs:34,:70,:98;encodings/fsst/src/array.rs:362encodings/datetime-parts/src/canonical.rs:69,:135i32before typed reads.encodings/onpair/src/canonical.rs:42,:73;encodings/onpair/src/decode.rs:51,:66vortex-arrow/src/executor/list.rs:97,:155,:162vortex-arrow/src/executor/byte.rs:108vortex-duckdb/src/exporter/list.rs:91;vortex-duckdb/src/exporter/list_view.rs:129vortex-duckdb/src/exporter/canonical.rs:26,:31vortex-cuda/src/kernel/arrays/list.rs:88;vortex-cuda/kernels/src/list.cu:36;vortex-cuda/kernels/src/list_view.cu:227vortex-cuda/src/kernel/encodings/fsst.rs:158,:202;vortex-cuda/kernels/src/fsst.cu:298vortex-cuda/kernels/src/arrow_offsets.cu:56vortex-cuda/kernels/src/onpair.cu:107Snapshots
compact/list_of_int_runsu16toi16.regular/list_of_int_runsu16toi16.regular/string_fsst_structuredu8toi8, code offsetsu32toi32.regular/string_low_cardinalityu8toi8.regular/struct_mixedu8toi8.Every stored range fits the signed type at the existing width. No byte-count changes need explanation.
Checks and compatibility
vortex-array,vortex-btrblocks,vortex-compressor, andvortex-fastlanes, with all targets, all features, and warnings denied: passed.nightly-2026-09-10for those four crates: passed.cargo nextest run -p vortex-array: 5,189 passed, one skipped.cargo nextest run -p vortex-btrblocks --all-features: 92 passed.cargo nextest run -p vortex-compressor -p vortex-fastlanes: 1,240 passed.cargo nextest run -p vortex-fsst -p vortex-onpair -p vortex-datetime-parts: 242 passed, one skipped.cargo nextest run -p vortex-file: 189 passed. No file-test changes were needed.U16and expectedI16. After migration, both cases passed..narrow(calls remain. All 24 migrated calls have the exact deprecation reason.git diff --checkpassed.0.90.0reader passed 36 fixtures, failed none, and skipped the same seven newer decimal fixtures for both base9bf403a1ceand WD6. The per-fixture results match exactly insubsetmode. All 43 fixtures were generated on each revision. The ClickBench download succeeded, and both runs reused the same deterministic sample. Both temporary compatibility worktrees were removed.b2bd758df8. Its results are pending.Performance evidence and remaining CI work
Local btrblocks comparisons were skipped at the maintainer's request. CodSpeed does not run the btrblocks benchmarks.
The valid WD3 report compares
7dd6ba4with2fb259b. All 12take_mapcases are classifiedNoChange, with latency differences from -1.03% to +0.96%. The valid WD4 report compares419b196with37031b1. All 12 cases areNoChange, with latency differences from -0.31% to +6.34%. WD4 carries a runner-difference warning. Neither report has an unexpected-base condition. WD1's comment uses head75c7f4a, while its final head isa56a48e, so it is excluded as evidence for the final PR.The
take_mapfixture usesu64indices and values directly and calls no narrowing code. The selectedhashbrown 0.17.1default builder usesfoldhash 0.2.0random state, with per-hasher and shared seeds. A fixed fixture RNG does not fix the hash layout. This establishes a possible source of variance, not the cause of the old regressions.WD6's own CodSpeed comparison, any needed flame graph analysis, and develop-history comparison remain pending the PR benchmark run. Only reports with the correct base and head will be used. No
take_mapcode changed.