Skip to content

Commit b980c09

Browse files
committed
Replace DynVarBinBuilder with a generic VarBinBuilder
`VarBinBuilder<O>` becomes an actual `ArrayBuilder`, carrying its own `DType` and offset width, so `DynVarBinBuilder` is no longer needed as a separate type-erasing wrapper. Encodings recover the concrete offset width from a `&mut dyn ArrayBuilder` with the new `match_each_varbin_builder!` (i32/i64) and `match_each_any_varbin_builder!` (every width) macros, which monomorphize the append body over the width instead of dispatching through `dyn`. The existing `append_to_builder` specializations for FSST, OnPair and Zstd are ported over mechanically; their optimizations follow in later commits. Because the builder's offset type now matches the Arrow target, the Arrow byte export hands the offsets buffer straight to Arrow with no cast, which also removes the `VarBinView` + `arrow_cast` round trip a `Utf8`/`Binary` dtype mismatch used to take. A `Binary` source exported to `Utf8` instead validates the bytes of its live values, skipping the extents of null slots that `GenericByteArray::try_new` would have rejected. Also adds `BinaryView::bytes`, which borrows from already-resolved buffer slices instead of cloning a buffer handle per row. Signed-off-by: Robert Kruszewski <robert@spiraldb.com>
1 parent e58fb58 commit b980c09

28 files changed

Lines changed: 1390 additions & 511 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

encodings/fsst/src/array.rs

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,15 @@ use vortex_array::arrays::VarBinArray;
3434
use vortex_array::arrays::varbin::VarBinArraySlotsExt;
3535
use vortex_array::buffer::BufferHandle;
3636
use vortex_array::builders::ArrayBuilder;
37-
use vortex_array::builders::DynVarBinBuilder;
37+
use vortex_array::builders::VarBinBuilder;
3838
use vortex_array::builders::VarBinViewBuilder;
3939
use vortex_array::dtype::DType;
40+
use vortex_array::dtype::IntegerPType;
4041
use vortex_array::dtype::Nullability;
4142
use vortex_array::dtype::PType;
4243
use vortex_array::legacy_session;
4344
use vortex_array::match_each_integer_ptype;
45+
use vortex_array::match_each_varbin_builder;
4446
use vortex_array::serde::ArrayChildren;
4547
use vortex_array::validity::Validity;
4648
use vortex_array::vtable::VTable;
@@ -307,23 +309,10 @@ impl VTable for FSST {
307309
builder: &mut dyn ArrayBuilder,
308310
ctx: &mut ExecutionCtx,
309311
) -> VortexResult<()> {
310-
if let Some(builder) = builder.as_any_mut().downcast_mut::<DynVarBinBuilder>() {
311-
let (bytes, lengths) = fsst_decode_bytes(array, ctx)?;
312-
let validity = array
313-
.array()
314-
.validity()?
315-
.execute_mask(array.array().len(), ctx)?;
316-
match_each_integer_ptype!(lengths.ptype(), |P| {
317-
builder.append_values(
318-
bytes.as_slice(),
319-
lengths.as_slice::<P>().iter().scan(0usize, |end, length| {
320-
*end += AsPrimitive::<usize>::as_(*length);
321-
Some(*end)
322-
}),
323-
&validity,
324-
)
325-
})?;
326-
return Ok(());
312+
if let Some(result) =
313+
match_each_varbin_builder!(builder, |builder| append_to_varbin(array, builder, ctx))
314+
{
315+
return result;
327316
}
328317

329318
let Some(builder) = builder.as_any_mut().downcast_mut::<VarBinViewBuilder>() else {
@@ -361,6 +350,32 @@ impl VTable for FSST {
361350
}
362351
}
363352

353+
/// Decompresses the values and appends them to `builder`.
354+
fn append_to_varbin<O: IntegerPType>(
355+
array: ArrayView<'_, FSST>,
356+
builder: &mut VarBinBuilder<O>,
357+
ctx: &mut ExecutionCtx,
358+
) -> VortexResult<()>
359+
where
360+
usize: AsPrimitive<O>,
361+
{
362+
let (bytes, lengths) = fsst_decode_bytes(array, ctx)?;
363+
let validity = array
364+
.array()
365+
.validity()?
366+
.execute_mask(array.array().len(), ctx)?;
367+
match_each_integer_ptype!(lengths.ptype(), |P| {
368+
builder.append_values(
369+
bytes.as_slice(),
370+
lengths.as_slice::<P>().iter().scan(0usize, |end, length| {
371+
*end += AsPrimitive::<usize>::as_(*length);
372+
Some(*end)
373+
}),
374+
&validity,
375+
)
376+
})
377+
}
378+
364379
#[array_slots(FSST)]
365380
pub struct FSSTSlots {
366381
/// Lengths of the original values before compression, can be compressed.

encodings/fsst/src/canonical.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ mod tests {
106106
use vortex_array::arrays::VarBinViewArray;
107107
use vortex_array::arrays::varbin::VarBinArrayExt;
108108
use vortex_array::builders::ArrayBuilder;
109-
use vortex_array::builders::DynVarBinBuilder;
109+
use vortex_array::builders::VarBinBuilder;
110110
use vortex_array::builders::VarBinViewBuilder;
111111
use vortex_array::dtype::DType;
112112
use vortex_array::dtype::Nullability;
@@ -212,7 +212,7 @@ mod tests {
212212

213213
{
214214
let mut builder =
215-
DynVarBinBuilder::with_capacity(chunked_arr.dtype().clone(), false, data.len());
215+
VarBinBuilder::<i32>::with_capacity(chunked_arr.dtype().clone(), data.len());
216216
chunked_arr
217217
.into_array()
218218
.append_to_builder(&mut builder, &mut ctx)?;

encodings/fsst/src/compress.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,11 @@ fn compress_views<O>(
199199
where
200200
O: IntegerPType + 'static,
201201
{
202-
let mut sink = FsstSink::<O>::with_capacity(strings.len(), compressor);
202+
let mut sink = FsstSink::<O>::with_capacity(
203+
DType::Binary(strings.dtype().nullability()),
204+
strings.len(),
205+
compressor,
206+
);
203207
let views = strings.views();
204208
let buffers = strings.data_buffers();
205209
match mask.bit_buffer() {
@@ -232,7 +236,11 @@ fn compress_varbin<O>(
232236
where
233237
O: IntegerPType + 'static,
234238
{
235-
let mut sink = FsstSink::<O>::with_capacity(strings.len(), compressor);
239+
let mut sink = FsstSink::<O>::with_capacity(
240+
DType::Binary(strings.dtype().nullability()),
241+
strings.len(),
242+
compressor,
243+
);
236244
let bytes = strings.bytes().as_slice();
237245
match_each_integer_ptype!(offsets.ptype(), |I| {
238246
let off = offsets.as_slice::<I>();
@@ -277,10 +285,10 @@ struct FsstSink<'c, O: IntegerPType + 'static> {
277285
}
278286

279287
impl<'c, O: IntegerPType + 'static> FsstSink<'c, O> {
280-
fn with_capacity(len: usize, compressor: &'c Compressor) -> Self {
288+
fn with_capacity(dtype: DType, len: usize, compressor: &'c Compressor) -> Self {
281289
Self {
282290
buffer: Vec::with_capacity(DEFAULT_BUFFER_LEN),
283-
builder: VarBinBuilder::<O>::with_capacity(len),
291+
builder: VarBinBuilder::<O>::with_capacity(dtype, len),
284292
uncompressed_lengths: BufferMut::with_capacity(len),
285293
compressor,
286294
}
@@ -289,7 +297,7 @@ impl<'c, O: IntegerPType + 'static> FsstSink<'c, O> {
289297
#[inline]
290298
fn emit(&mut self, row: Option<&[u8]>) {
291299
let Some(s) = row else {
292-
self.builder.append_null();
300+
self.builder.push_null();
293301
self.uncompressed_lengths.push(0);
294302
return;
295303
};
@@ -310,8 +318,8 @@ impl<'c, O: IntegerPType + 'static> FsstSink<'c, O> {
310318
self.builder.append_value(&self.buffer);
311319
}
312320

313-
fn finish(self, dtype: DType, ctx: &mut ExecutionCtx) -> VortexResult<FSSTArray> {
314-
let codes = self.builder.finish(DType::Binary(dtype.nullability()));
321+
fn finish(mut self, dtype: DType, ctx: &mut ExecutionCtx) -> VortexResult<FSSTArray> {
322+
let codes = self.builder.finish_into_varbin();
315323
FSST::try_new(
316324
dtype,
317325
Buffer::copy_from(self.compressor.symbol_table()),

encodings/fsst/src/kernel.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ mod tests {
6161
});
6262

6363
fn build_test_fsst_array() -> ArrayRef {
64-
let mut builder = VarBinBuilder::<i32>::with_capacity(10);
64+
let mut builder =
65+
VarBinBuilder::<i32>::with_capacity(DType::Utf8(Nullability::NonNullable), 10);
6566
builder.append_value(b"hello world");
6667
builder.append_value(b"foo bar baz");
6768
builder.append_value(b"testing fsst compression");
@@ -72,7 +73,7 @@ mod tests {
7273
builder.append_value(b"qrstuvwxyz");
7374
builder.append_value(b"0123456789");
7475
builder.append_value(b"final string");
75-
let input = builder.finish(DType::Utf8(Nullability::NonNullable));
76+
let input = builder.finish_into_varbin();
7677

7778
let mut ctx = SESSION.create_execution_ctx();
7879
let arr = input.into_array();
@@ -131,7 +132,8 @@ mod tests {
131132
// Test case with special characters and nulls
132133
// Values: ["", "", "", "", "", "", "", "", "", "", "", ",", "A<<<<<<<", "", "", "", "", null, null, null, null, null, null]
133134
// Mask: only the last element is selected (true at index 22)
134-
let mut builder = VarBinBuilder::<i32>::with_capacity(23);
135+
let mut builder =
136+
VarBinBuilder::<i32>::with_capacity(DType::Utf8(Nullability::Nullable), 23);
135137
// 11 empty strings
136138
for _ in 0..11 {
137139
builder.append_value(b"");
@@ -146,9 +148,9 @@ mod tests {
146148
}
147149
// 6 nulls
148150
for _ in 0..6 {
149-
builder.append_null();
151+
builder.push_null();
150152
}
151-
let input = builder.finish(DType::Utf8(Nullability::Nullable));
153+
let input = builder.finish_into_varbin();
152154
let array = input.clone().into_array();
153155

154156
let mut ctx = SESSION.create_execution_ctx();
@@ -172,12 +174,13 @@ mod tests {
172174

173175
#[test]
174176
fn filter_only_null() -> VortexResult<()> {
175-
let mut builder = VarBinBuilder::<i32>::with_capacity(3);
176-
builder.append_null();
177+
let mut builder =
178+
VarBinBuilder::<i32>::with_capacity(DType::Utf8(Nullability::Nullable), 3);
179+
builder.push_null();
177180
builder.append_value(b"A");
178-
builder.append_null();
181+
builder.push_null();
179182

180-
let input = builder.finish(DType::Utf8(Nullability::Nullable));
183+
let input = builder.finish_into_varbin();
181184
let array = input.clone().into_array();
182185

183186
let mut ctx = SESSION.create_execution_ctx();
@@ -213,15 +216,14 @@ mod tests {
213216

214217
#[test]
215218
fn test_fsst_byte_length() -> VortexResult<()> {
216-
let mut builder = VarBinBuilder::<i32>::with_capacity(3);
219+
let mut builder =
220+
VarBinBuilder::<i32>::with_capacity(DType::Utf8(Nullability::NonNullable), 3);
217221
builder.append_value(b"hello");
218222
builder.append_value(b"world!!");
219223
builder.append_value("Пуховички"); // 9 characters, 18 bytes
220224
builder.append_value(b"");
221225

222-
let varbin = builder
223-
.finish(DType::Utf8(Nullability::NonNullable))
224-
.into_array();
226+
let varbin = builder.finish_into_varbin().into_array();
225227
let mut ctx = SESSION.create_execution_ctx();
226228
let compressor = fsst_train_compressor(&varbin, &mut ctx)?;
227229
let fsst = fsst_compress(&varbin, &compressor, &mut ctx)?.into_array();

encodings/fsst/src/tests.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,14 @@ static SESSION: LazyLock<VortexSession> = LazyLock::new(|| {
3030

3131
/// this function is VERY slow on miri, so we only want to run it once
3232
pub(crate) fn build_fsst_array(ctx: &mut ExecutionCtx) -> ArrayRef {
33-
let mut input_array = VarBinBuilder::<i32>::with_capacity(3);
33+
let mut input_array =
34+
VarBinBuilder::<i32>::with_capacity(DType::Utf8(Nullability::NonNullable), 3);
3435
input_array.append_value(b"The Greeks never said that the limit could not be overstepped");
3536
input_array.append_value(
3637
b"They said it existed and that whoever dared to exceed it was mercilessly struck down",
3738
);
3839
input_array.append_value(b"Nothing in present history can contradict them");
39-
let input_array = input_array
40-
.finish(DType::Utf8(Nullability::NonNullable))
41-
.into_array();
40+
let input_array = input_array.finish_into_varbin().into_array();
4241

4342
let compressor = fsst_train_compressor(&input_array, ctx).unwrap();
4443
fsst_compress(&input_array, &compressor, ctx)
@@ -162,13 +161,11 @@ fn fsst_compress_offsets_overflow_i32() {
162161

163162
println!("building large VarBinArray");
164163
let string = vec![b'a'; STRING_LEN];
165-
let mut builder = VarBinBuilder::<i64>::with_capacity(N);
164+
let mut builder = VarBinBuilder::<i64>::with_capacity(DType::Utf8(Nullability::NonNullable), N);
166165
for _ in 0..N {
167166
builder.append_value(&string);
168167
}
169-
let array = builder
170-
.finish(DType::Utf8(Nullability::NonNullable))
171-
.into_array();
168+
let array = builder.finish_into_varbin().into_array();
172169

173170
let compressor = CompressorBuilder::default().build();
174171
let len = array.len();

encodings/onpair/src/array.rs

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ use vortex_array::IntoArray;
2626
use vortex_array::array_slots;
2727
use vortex_array::buffer::BufferHandle;
2828
use vortex_array::builders::ArrayBuilder;
29-
use vortex_array::builders::DynVarBinBuilder;
29+
use vortex_array::builders::VarBinBuilder;
3030
use vortex_array::builders::VarBinViewBuilder;
3131
use vortex_array::dtype::DType;
32+
use vortex_array::dtype::IntegerPType;
3233
use vortex_array::dtype::Nullability;
3334
use vortex_array::dtype::PType;
3435
use vortex_array::match_each_integer_ptype;
36+
use vortex_array::match_each_varbin_builder;
3537
use vortex_array::serde::ArrayChildren;
3638
use vortex_array::validity::Validity;
3739
use vortex_array::vtable::VTable;
@@ -553,23 +555,10 @@ impl VTable for OnPair {
553555
builder: &mut dyn ArrayBuilder,
554556
ctx: &mut ExecutionCtx,
555557
) -> VortexResult<()> {
556-
if let Some(builder) = builder.as_any_mut().downcast_mut::<DynVarBinBuilder>() {
557-
let (bytes, lengths) = onpair_decode_bytes(array, ctx)?;
558-
let validity = array
559-
.array()
560-
.validity()?
561-
.execute_mask(array.array().len(), ctx)?;
562-
match_each_integer_ptype!(lengths.ptype(), |P| {
563-
builder.append_values(
564-
bytes.as_slice(),
565-
lengths.as_slice::<P>().iter().scan(0usize, |end, length| {
566-
*end += AsPrimitive::<usize>::as_(*length);
567-
Some(*end)
568-
}),
569-
&validity,
570-
)
571-
})?;
572-
return Ok(());
558+
if let Some(result) =
559+
match_each_varbin_builder!(builder, |builder| append_to_varbin(array, builder, ctx))
560+
{
561+
return result;
573562
}
574563

575564
let Some(builder) = builder.as_any_mut().downcast_mut::<VarBinViewBuilder>() else {
@@ -603,6 +592,32 @@ impl VTable for OnPair {
603592
}
604593
}
605594

595+
/// Decodes the values and appends them to `builder`.
596+
fn append_to_varbin<O: IntegerPType>(
597+
array: ArrayView<'_, OnPair>,
598+
builder: &mut VarBinBuilder<O>,
599+
ctx: &mut ExecutionCtx,
600+
) -> VortexResult<()>
601+
where
602+
usize: AsPrimitive<O>,
603+
{
604+
let (bytes, lengths) = onpair_decode_bytes(array, ctx)?;
605+
let validity = array
606+
.array()
607+
.validity()?
608+
.execute_mask(array.array().len(), ctx)?;
609+
match_each_integer_ptype!(lengths.ptype(), |P| {
610+
builder.append_values(
611+
bytes.as_slice(),
612+
lengths.as_slice::<P>().iter().scan(0usize, |end, length| {
613+
*end += AsPrimitive::<usize>::as_(*length);
614+
Some(*end)
615+
}),
616+
&validity,
617+
)
618+
})
619+
}
620+
606621
impl ValidityVTable<OnPair> for OnPair {
607622
fn validity(array: ArrayView<'_, OnPair>) -> VortexResult<Validity> {
608623
Ok(child_to_validity(

encodings/onpair/src/tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use vortex_array::arrays::VarBinViewArray;
1616
use vortex_array::arrays::filter::FilterKernel;
1717
use vortex_array::assert_arrays_eq;
1818
use vortex_array::buffer::BufferHandle;
19-
use vortex_array::builders::DynVarBinBuilder;
19+
use vortex_array::builders::VarBinBuilder;
2020
use vortex_array::builtins::ArrayBuiltins;
2121
use vortex_array::dtype::DType;
2222
use vortex_array::dtype::Nullability;
@@ -66,7 +66,7 @@ fn test_direct_offset_builder() -> vortex_error::VortexResult<()> {
6666
let mut ctx = SESSION.create_execution_ctx();
6767
let input = sample_input();
6868
let encoded = compress_onpair(input.as_ref(), &mut ctx)?;
69-
let mut builder = DynVarBinBuilder::with_capacity(input.dtype().clone(), false, input.len());
69+
let mut builder = VarBinBuilder::<i32>::with_capacity(input.dtype().clone(), input.len());
7070
encoded
7171
.into_array()
7272
.append_to_builder(&mut builder, &mut ctx)?;

encodings/runend/src/array.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ mod tests {
506506
use vortex_array::arrays::DictArray;
507507
use vortex_array::arrays::VarBinViewArray;
508508
use vortex_array::assert_arrays_eq;
509-
use vortex_array::builders::DynVarBinBuilder;
509+
use vortex_array::builders::VarBinBuilder;
510510
use vortex_array::dtype::DType;
511511
use vortex_array::dtype::Nullability;
512512
use vortex_array::dtype::PType;
@@ -564,7 +564,7 @@ mod tests {
564564
Some("c"),
565565
])
566566
.into_array();
567-
let mut builder = DynVarBinBuilder::with_capacity(arr.dtype().clone(), false, arr.len());
567+
let mut builder = VarBinBuilder::<i32>::with_capacity(arr.dtype().clone(), arr.len());
568568
arr.append_to_builder(&mut builder, &mut ctx).unwrap();
569569
assert_arrays_eq!(builder.finish_into_varbin(), expected, &mut ctx);
570570
assert_arrays_eq!(arr.into_array(), expected, &mut ctx);

0 commit comments

Comments
 (0)