Skip to content

Commit e3afa88

Browse files
feat(vortex-onpair): store codes_offsets at adaptive u32/u64 width
Compression accepts u64 byte offsets (large-binary inputs), but the codes_offsets child was always narrowed to u32, capping a chunk at 2^32 tokens and failing compression above it. Pick the narrowest of u32/u64 that holds the largest per-row code boundary instead, so codes_offsets scales with the u64 byte-offset capacity. The cascading compressor still narrows the common u32 case down to u16/u8, and the width round-trips via the existing codes_offsets_ptype metadata, so the serialized format is unchanged. Widen CodesWindow to Buffer<u64> to match; the u64->usize conversions are checked (cast_possible_truncation is denied) but fold away on 64-bit. The u64 branch cannot be reached with realistic test data (>4 GiB chunk), so cover it two ways: a unit test drives the width selection via a threshold parameter, and a read-path test hand-widens a small array's codes_offsets child and asserts canonical decode and the compressed-domain equality compare (CodesWindow) behave identically to the u32 width. Signed-off-by: Francesco Gargiulo <francesco@spiraldb.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b8f158d commit e3afa88

4 files changed

Lines changed: 137 additions & 20 deletions

File tree

encodings/experimental/onpair/src/array.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,9 @@ pub struct OnPairSlots {
104104
/// Primitive integer token codes. Downstream integer compression may
105105
/// narrow or bit-pack this child independently of the OnPair metadata.
106106
pub codes: ArrayRef,
107-
/// `PrimitiveArray<u32>`, length `num_rows + 1`. FoR / RunEnd / etc. apply
108-
/// naturally via the cascading compressor.
107+
/// `PrimitiveArray<u32>` (or `u64` when a chunk exceeds `u32::MAX` tokens),
108+
/// length `num_rows + 1`. FoR / RunEnd / etc. apply naturally via the
109+
/// cascading compressor.
109110
pub codes_offsets: ArrayRef,
110111
/// Integer `PrimitiveArray`, length `num_rows`. Used to size the canonical
111112
/// output buffer.

encodings/experimental/onpair/src/compress.rs

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -88,17 +88,7 @@ where
8888
.map_err(|e| vortex_err!("OnPair compress failed: {e}"))?;
8989
let (dict, codes, row_offsets) = column.into_raw();
9090
let (dict_bytes, dict_offsets) = dict.into_raw();
91-
let codes_offsets = Buffer::from(
92-
row_offsets
93-
.into_iter()
94-
.map(|o| {
95-
let value = o.to_usize();
96-
u32::try_from(value)
97-
.map_err(|_| vortex_err!("OnPair code boundary {value} does not fit u32"))
98-
})
99-
.collect::<VortexResult<Vec<_>>>()?,
100-
)
101-
.into_array();
91+
let codes_offsets = codes_offsets_array(&row_offsets, u32::MAX as usize);
10292
let codes = Buffer::from(codes).into_array();
10393
let dict_offsets = Buffer::from(dict_offsets).into_array();
10494

@@ -133,6 +123,35 @@ fn dict_bytes_to_buffer(dict_bytes: Vec<u8>) -> BufferHandle {
133123
BufferHandle::new_host(aligned.freeze())
134124
}
135125

126+
/// Build the `codes_offsets` child from the library's per-row code boundaries,
127+
/// storing the narrowest of `u32`/`u64` that holds the largest boundary.
128+
/// `row_offsets` is non-decreasing, so its last entry is that maximum and one
129+
/// bound check picks the width. `u32` covers the common case (the cascading
130+
/// compressor narrows it further to `u16`/`u8`); `u64` engages only when a
131+
/// single chunk carries more than `u32_max` tokens, matching the `u64` byte
132+
/// offsets accepted at compression. `u32_max` is a parameter so tests can drive
133+
/// the `u64` branch without a multi-GiB array.
134+
fn codes_offsets_array<O: Offset>(row_offsets: &[O], u32_max: usize) -> ArrayRef {
135+
let total_tokens = row_offsets.last().map_or(0, |&o| o.to_usize());
136+
if total_tokens <= u32_max {
137+
Buffer::from(
138+
row_offsets
139+
.iter()
140+
.map(|&o| u32::try_from(o.to_usize()).vortex_expect("code boundary fits u32"))
141+
.collect::<Vec<u32>>(),
142+
)
143+
.into_array()
144+
} else {
145+
Buffer::from(
146+
row_offsets
147+
.iter()
148+
.map(|&o| u64::try_from(o.to_usize()).vortex_expect("token count fits u64"))
149+
.collect::<Vec<u64>>(),
150+
)
151+
.into_array()
152+
}
153+
}
154+
136155
/// Compress any [`ArrayRef`] whose canonical form is a string array, by first
137156
/// canonicalising to `VarBinViewArray`.
138157
pub fn onpair_compress(
@@ -143,3 +162,30 @@ pub fn onpair_compress(
143162
let view = array.clone().execute::<VarBinViewArray>(ctx)?;
144163
onpair_compress_varbinview::<u64>(view, config, ctx)
145164
}
165+
166+
#[cfg(test)]
167+
mod tests {
168+
use vortex_array::dtype::DType;
169+
use vortex_array::dtype::Nullability;
170+
use vortex_array::dtype::PType;
171+
172+
use super::codes_offsets_array;
173+
174+
#[test]
175+
fn codes_offsets_width_selection() {
176+
// Largest boundary within the threshold is stored as u32.
177+
let narrow = codes_offsets_array::<u64>(&[0, 3, 7], 7);
178+
assert_eq!(narrow.len(), 3);
179+
assert_eq!(
180+
narrow.dtype(),
181+
&DType::Primitive(PType::U32, Nullability::NonNullable)
182+
);
183+
184+
// A boundary above the threshold widens the child to u64.
185+
let wide = codes_offsets_array::<u64>(&[0, 3, 8], 7);
186+
assert_eq!(
187+
wide.dtype(),
188+
&DType::Primitive(PType::U64, Nullability::NonNullable)
189+
);
190+
}
191+
}

encodings/experimental/onpair/src/decode.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use vortex_array::builtins::ArrayBuiltins;
1212
use vortex_array::dtype::DType;
1313
use vortex_array::dtype::NativePType;
1414
use vortex_buffer::Buffer;
15+
use vortex_error::VortexExpect;
1516
use vortex_error::VortexResult;
1617
use vortex_error::vortex_ensure;
1718
use vortex_error::vortex_err;
@@ -57,17 +58,21 @@ pub(crate) fn code_boundary_at(
5758
///
5859
/// [`row`]: CodesWindow::row
5960
pub(crate) struct CodesWindow {
60-
offsets: Buffer<u32>,
61+
offsets: Buffer<u64>,
6162
codes: Buffer<u16>,
6263
code_start: usize,
6364
}
6465

6566
impl CodesWindow {
6667
/// The codes for row `i`.
6768
pub(crate) fn row(&self, i: usize) -> &[u16] {
68-
let start = self.offsets[i] as usize - self.code_start;
69-
let end = self.offsets[i + 1] as usize - self.code_start;
70-
&self.codes[start..end]
69+
&self.codes[self.local(i)..self.local(i + 1)]
70+
}
71+
72+
/// Offset `i` rebased into the window's local `codes` slice. Offsets are
73+
/// bounded by `codes.len()` (a `usize`), so the conversion never truncates.
74+
fn local(&self, i: usize) -> usize {
75+
usize::try_from(self.offsets[i]).vortex_expect("code offset fits usize") - self.code_start
7176
}
7277
}
7378

@@ -80,7 +85,7 @@ pub(crate) fn collect_codes_window(
8085
ctx: &mut ExecutionCtx,
8186
) -> VortexResult<CodesWindow> {
8287
let len = array.len();
83-
let offsets = collect_widened::<u32>(array.codes_offsets(), ctx)?;
88+
let offsets = collect_widened::<u64>(array.codes_offsets(), ctx)?;
8489
vortex_ensure!(
8590
offsets.len() == len + 1,
8691
"OnPair codes_offsets has {} entries, expected len + 1 = {}",
@@ -91,8 +96,8 @@ pub(crate) fn collect_codes_window(
9196
offsets.is_sorted(),
9297
"OnPair codes_offsets must be nondecreasing"
9398
);
94-
let code_start = offsets[0] as usize;
95-
let code_end = offsets[len] as usize;
99+
let code_start = usize::try_from(offsets[0]).vortex_expect("code offset fits usize");
100+
let code_end = usize::try_from(offsets[len]).vortex_expect("code offset fits usize");
96101
vortex_ensure!(
97102
code_end <= array.codes().len(),
98103
"OnPair codes_offsets end {} exceeds codes len {}",

encodings/experimental/onpair/src/tests.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,19 @@ use onpair::CompactDictionaryView;
77
use prost::Message;
88
use vortex_array::IntoArray;
99
use vortex_array::VortexSessionExecute;
10+
use vortex_array::arrays::BoolArray;
11+
use vortex_array::arrays::ConstantArray;
1012
use vortex_array::arrays::PrimitiveArray;
1113
use vortex_array::arrays::VarBinArray;
1214
use vortex_array::arrays::VarBinViewArray;
1315
use vortex_array::arrays::filter::FilterKernel;
1416
use vortex_array::assert_arrays_eq;
17+
use vortex_array::builtins::ArrayBuiltins;
1518
use vortex_array::dtype::DType;
1619
use vortex_array::dtype::Nullability;
1720
use vortex_array::dtype::PType;
1821
use vortex_array::match_each_integer_ptype;
22+
use vortex_array::scalar_fn::fns::operators::Operator;
1923
use vortex_array::test_harness::check_metadata;
2024
use vortex_array::validity::Validity;
2125
use vortex_buffer::BufferMut;
@@ -114,6 +118,67 @@ fn test_onpair_roundtrip() -> vortex_error::VortexResult<()> {
114118
Ok(())
115119
}
116120

121+
/// The `u64` `codes_offsets` branch only engages past `u32::MAX` tokens (a
122+
/// multi-GiB chunk), so exercise the read path by widening a small array's
123+
/// `codes_offsets` child to `u64` by hand and asserting the decode paths
124+
/// (canonical and the compressed-domain equality compare, which builds a
125+
/// `CodesWindow`) treat it identically to the default `u32` width.
126+
#[cfg_attr(miri, ignore)]
127+
#[test]
128+
fn test_onpair_u64_codes_offsets() -> vortex_error::VortexResult<()> {
129+
let mut ctx = SESSION.create_execution_ctx();
130+
let narrow = onpair_compress(
131+
&sample_input().into_array(),
132+
DEFAULT_DICT12_CONFIG,
133+
&mut ctx,
134+
)?;
135+
136+
// Rebuild with only codes_offsets widened to u64; every other child is the
137+
// input's, so the two arrays differ solely in codes_offsets width.
138+
let wide = {
139+
let view = narrow.as_view();
140+
let wide_offsets = view
141+
.codes_offsets()
142+
.cast(DType::Primitive(PType::U64, Nullability::NonNullable))?
143+
.execute::<PrimitiveArray>(&mut ctx)?
144+
.into_array();
145+
OnPair::try_new(
146+
view.dtype().clone(),
147+
view.dict_bytes_handle().clone(),
148+
view.dict_offsets().clone(),
149+
view.codes().clone(),
150+
wide_offsets,
151+
view.uncompressed_lengths().clone(),
152+
view.array_validity(),
153+
)?
154+
};
155+
assert_eq!(
156+
wide.as_view().codes_offsets().dtype(),
157+
&DType::Primitive(PType::U64, Nullability::NonNullable)
158+
);
159+
160+
// Canonical decode is byte-identical across the two widths.
161+
let narrow_decoded = narrow.into_array().execute::<VarBinViewArray>(&mut ctx)?;
162+
let wide_decoded = wide
163+
.clone()
164+
.into_array()
165+
.execute::<VarBinViewArray>(&mut ctx)?;
166+
assert_arrays_eq!(&wide_decoded, &narrow_decoded, &mut ctx);
167+
168+
// Equality compare drives CodesWindow over the u64 codes_offsets.
169+
let needle = ConstantArray::new("https://www.example.com/page", wide.len()).into_array();
170+
let eq = wide
171+
.into_array()
172+
.binary(needle, Operator::Eq)?
173+
.execute::<BoolArray>(&mut ctx)?;
174+
assert_arrays_eq!(
175+
&eq,
176+
&BoolArray::from_iter([true, false, false, false, true]),
177+
&mut ctx
178+
);
179+
Ok(())
180+
}
181+
117182
#[cfg_attr(miri, ignore)]
118183
#[test]
119184
fn test_onpair_nullable_canonicalize() -> vortex_error::VortexResult<()> {

0 commit comments

Comments
 (0)