Skip to content

Commit d9583d3

Browse files
committed
Use u8 Union type IDs
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
1 parent 01e5eb0 commit d9583d3

7 files changed

Lines changed: 59 additions & 61 deletions

File tree

‎vortex-array/src/dtype/serde/flatbuffers.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ impl UnionVariants {
117117
})
118118
.collect::<Vec<_>>();
119119

120-
let type_ids: Vec<i8> = fb_union
120+
let type_ids: Vec<u8> = fb_union
121121
.type_ids()
122122
.ok_or_else(|| vortex_err!("failed to parse union type_ids from flatbuffer"))?
123123
.iter()
@@ -687,7 +687,7 @@ mod test {
687687
.unwrap();
688688
let dtypes = fbb.create_vector(&[dtype_a, dtype_b]);
689689

690-
let type_ids = fbb.create_vector::<i8>(&[0]);
690+
let type_ids = fbb.create_vector::<u8>(&[0]);
691691

692692
let union_table = fb::Union::create(
693693
&mut fbb,

‎vortex-array/src/dtype/serde/proto.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ impl DType {
9898
.type_ids
9999
.iter()
100100
.map(|t| {
101-
i8::try_from(*t).map_err(|_| {
102-
vortex_err!("Union type_id {t} somehow does not fit in i8")
101+
u8::try_from(*t).map_err(|_| {
102+
vortex_err!("Union type_id {t} somehow does not fit in u8")
103103
})
104104
})
105105
.collect::<VortexResult<Vec<_>>>()?;
@@ -469,8 +469,8 @@ mod tests {
469469
pb::DType::try_from(&DType::Primitive(PType::I32, Nullability::NonNullable))
470470
.unwrap(),
471471
],
472-
// 200 does not fit in i8.
473-
type_ids: vec![200],
472+
// 256 does not fit in u8.
473+
type_ids: vec![256],
474474
})),
475475
};
476476

@@ -480,7 +480,7 @@ mod tests {
480480
result
481481
.unwrap_err()
482482
.to_string()
483-
.contains("does not fit in i8")
483+
.contains("does not fit in u8")
484484
);
485485
}
486486

‎vortex-array/src/dtype/serde/serde.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ impl<'de> DeserializeSeed<'de> for DTypeSerde<'_, UnionVariants> {
431431
{
432432
let mut names: Option<FieldNames> = None;
433433
let mut dtypes: Option<Vec<DType>> = None;
434-
let mut type_ids: Option<Vec<i8>> = None;
434+
let mut type_ids: Option<Vec<u8>> = None;
435435

436436
while let Some(key) = map.next_key::<Cow<'_, str>>()? {
437437
match key.as_ref() {

‎vortex-array/src/dtype/union.rs‎

Lines changed: 44 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,11 @@ use crate::dtype::Nullability;
1919
/// Type information for a union array.
2020
///
2121
/// A `UnionVariants` describes the possible alternative types for each row of a union, along with a
22-
/// per-variant `i8` type tag. We use the term **variants** (rather than "fields") because a union
22+
/// per-variant `u8` type tag. We use the term **variants** (rather than "fields") because a union
2323
/// is a sum type: each row chooses exactly one alternative.
2424
///
25-
/// Per Arrow's spec, the per-row type tag is an `int8`. By default, tag `i` selects the child at
26-
/// offset `i` (`type_ids = [0, 1, ..., N-1]`).
27-
///
28-
/// Type IDs are restricted to the Arrow-compatible range `0..=127`; negative tags are rejected.
25+
/// By default, tag `i` selects the child at offset `i` (`type_ids = [0, 1, ..., N-1]`). Vortex uses
26+
/// unsigned tags and supports up to 255 variants.
2927
///
3028
/// Schemas may also use non-consecutive tags (e.g. `[0, 5, 7]`), in which case the value of
3129
/// `type_ids[i]` is the tag used in the data to select the child at offset `i`. Supporting
@@ -114,11 +112,11 @@ struct UnionVariantsInner {
114112
/// consecutive offsets is `[0, 1, ..., N-1]`.
115113
///
116114
/// For schemas with explicit `typeIds` indirection (e.g. `[0, 5, 7]`), this stores those tags.
117-
type_ids: Arc<[i8]>,
115+
type_ids: Arc<[u8]>,
118116
}
119117

120118
impl UnionVariantsInner {
121-
fn from_fields(names: FieldNames, dtypes: Arc<[FieldDType]>, type_ids: Arc<[i8]>) -> Self {
119+
fn from_fields(names: FieldNames, dtypes: Arc<[FieldDType]>, type_ids: Arc<[u8]>) -> Self {
122120
Self {
123121
names,
124122
dtypes,
@@ -160,7 +158,7 @@ impl UnionVariants {
160158
}
161159

162160
/// Validate that `names`, `dtypes`, and `type_ids` are mutually consistent.
163-
fn validate_shape(names: &FieldNames, n_dtypes: usize, type_ids: &[i8]) -> VortexResult<()> {
161+
fn validate_shape(names: &FieldNames, n_dtypes: usize, type_ids: &[u8]) -> VortexResult<()> {
164162
vortex_ensure_eq!(
165163
names.len(),
166164
n_dtypes,
@@ -182,9 +180,10 @@ impl UnionVariants {
182180
type_ids
183181
);
184182
vortex_ensure!(
185-
type_ids.iter().all(|type_id| *type_id >= 0),
186-
"type_ids must be non-negative, got {:?}",
187-
type_ids
183+
type_ids.len() <= u8::MAX as usize,
184+
"union supports at most {} variants, got {}",
185+
u8::MAX,
186+
type_ids.len()
188187
);
189188
vortex_ensure!(
190189
names.iter().all_unique(),
@@ -200,12 +199,12 @@ impl UnionVariants {
200199
/// # Errors
201200
///
202201
/// Returns an error if names, dtypes, or type IDs do not all have the same length, or if there
203-
/// are any duplicate names or type ids, or if a type ID is negative.
204-
pub fn try_new(names: FieldNames, dtypes: Vec<DType>, type_ids: Vec<i8>) -> VortexResult<Self> {
202+
/// are any duplicate names or type IDs, or if there are more than 255 variants.
203+
pub fn try_new(names: FieldNames, dtypes: Vec<DType>, type_ids: Vec<u8>) -> VortexResult<Self> {
205204
Self::validate_shape(&names, dtypes.len(), &type_ids)?;
206205

207206
let dtypes: Arc<[FieldDType]> = dtypes.into_iter().map(FieldDType::from).collect();
208-
let type_ids: Arc<[i8]> = Arc::from(type_ids);
207+
let type_ids: Arc<[u8]> = Arc::from(type_ids);
209208

210209
Ok(Self(Arc::new(UnionVariantsInner::from_fields(
211210
names, dtypes, type_ids,
@@ -217,21 +216,21 @@ impl UnionVariants {
217216
/// # Errors
218217
///
219218
/// `names` and `dtypes` must have the same length, and `names.len()` cannot be more than
220-
/// `i8::MAX as usize + 1` (128).
219+
/// `u8::MAX as usize` (255).
221220
pub fn new(names: FieldNames, dtypes: Vec<DType>) -> VortexResult<Self> {
222-
const MAX_CONSECUTIVE: usize = i8::MAX as usize + 1;
221+
const MAX_VARIANTS: usize = u8::MAX as usize;
223222
vortex_ensure!(
224-
names.len() <= MAX_CONSECUTIVE,
223+
names.len() <= MAX_VARIANTS,
225224
"union supports at most {} consecutive variants, got {}",
226-
MAX_CONSECUTIVE,
225+
MAX_VARIANTS,
227226
names.len()
228227
);
229228

230229
#[expect(
231230
clippy::cast_possible_truncation,
232-
reason = "the MAX_CONSECUTIVE bound above guarantees `i as i8` is in range"
231+
reason = "the MAX_VARIANTS bound above guarantees `i as u8` is in range"
233232
)]
234-
let type_ids: Vec<i8> = (0..names.len()).map(|i| i as i8).collect();
233+
let type_ids: Vec<u8> = (0..names.len()).map(|i| i as u8).collect();
235234

236235
Self::try_new(names, dtypes, type_ids)
237236
}
@@ -244,11 +243,11 @@ impl UnionVariants {
244243
/// # Errors
245244
///
246245
/// Returns an error if names, dtypes, or type IDs do not all have the same length, or if there
247-
/// are any duplicate names or type ids, or if a type ID is negative.
246+
/// are any duplicate names or type IDs, or if there are more than 255 variants.
248247
pub(crate) fn try_from_fields(
249248
names: FieldNames,
250249
dtypes: Vec<FieldDType>,
251-
type_ids: Vec<i8>,
250+
type_ids: Vec<u8>,
252251
) -> VortexResult<Self> {
253252
Self::validate_shape(&names, dtypes.len(), &type_ids)?;
254253

@@ -276,7 +275,7 @@ impl UnionVariants {
276275

277276
/// Returns the per-variant type tag vector. Entry `i` is the tag that the data uses to
278277
/// select the variant at offset `i`.
279-
pub fn type_ids(&self) -> &[i8] {
278+
pub fn type_ids(&self) -> &[u8] {
280279
&self.0.type_ids
281280
}
282281

@@ -286,12 +285,12 @@ impl UnionVariants {
286285
.type_ids
287286
.iter()
288287
.enumerate()
289-
.all(|(i, &tag)| i8::try_from(i).is_ok_and(|i| i == tag))
288+
.all(|(i, &tag)| u8::try_from(i).is_ok_and(|i| i == tag))
290289
}
291290

292291
/// Find the offset of a variant by name. Returns `None` if no variant has the name.
293292
///
294-
/// This is a linear scan over [`Self::names`]. A union has at most 128 variants (and usually
293+
/// This is a linear scan over [`Self::names`]. A union has at most 255 variants (and usually
295294
/// far fewer), so a linear scan beats building and probing a `HashMap` in practice.
296295
pub fn find(&self, name: impl AsRef<str>) -> Option<usize> {
297296
let name = name.as_ref();
@@ -331,9 +330,9 @@ impl UnionVariants {
331330
/// `type_ids`.
332331
///
333332
/// This is a linear scan over [`Self::type_ids`]. The number of variants is bounded by
334-
/// `i8::MAX + 1 = 128`, which fits in two cache lines, so a linear scan is faster than a
335-
/// `HashMap` lookup in practice.
336-
pub fn tag_to_child_index(&self, tag: i8) -> Option<usize> {
333+
/// `u8::MAX = 255`, so a linear scan is faster than a `HashMap` lookup in practice for the
334+
/// small unions typically encountered.
335+
pub fn tag_to_child_index(&self, tag: u8) -> Option<usize> {
337336
self.0.type_ids.iter().position(|&t| t == tag)
338337
}
339338

@@ -342,7 +341,7 @@ impl UnionVariants {
342341
/// # Panics
343342
///
344343
/// Panics if `index >= self.len()`.
345-
pub fn child_index_to_tag(&self, index: usize) -> i8 {
344+
pub fn child_index_to_tag(&self, index: usize) -> u8 {
346345
self.0.type_ids[index]
347346
}
348347

@@ -437,14 +436,14 @@ mod tests {
437436
}
438437

439438
#[test]
440-
fn test_negative_type_ids_rejected() {
441-
let result = UnionVariants::try_new(
442-
["negative"].into(),
439+
fn test_high_type_id() {
440+
let variants = UnionVariants::try_new(
441+
["high"].into(),
443442
vec![DType::Primitive(PType::I32, Nullability::NonNullable)],
444-
vec![-1],
445-
);
446-
assert!(result.is_err());
447-
assert!(result.unwrap_err().to_string().contains("non-negative"));
443+
vec![u8::MAX],
444+
)
445+
.unwrap();
446+
assert_eq!(variants.type_ids(), &[u8::MAX]);
448447
}
449448

450449
#[test]
@@ -575,23 +574,22 @@ mod tests {
575574

576575
#[test]
577576
fn test_new_max_size() {
578-
// 128 variants is the maximum for consecutive type_ids: tags 0..=127 all fit in i8.
579-
let names: Vec<String> = (0..128).map(|i| format!("v{i}")).collect();
580-
let dtypes: Vec<DType> = (0..128)
577+
// 255 variants is the maximum supported size.
578+
let names: Vec<String> = (0..255).map(|i| format!("v{i}")).collect();
579+
let dtypes: Vec<DType> = (0..255)
581580
.map(|_| DType::Primitive(PType::I32, Nullability::NonNullable))
582581
.collect();
583582
let names: FieldNames = names.into_iter().collect();
584583
let variants = UnionVariants::new(names, dtypes).unwrap();
585-
assert_eq!(variants.len(), 128);
586-
assert_eq!(variants.type_ids()[127], 127);
584+
assert_eq!(variants.len(), 255);
585+
assert_eq!(variants.type_ids()[254], 254);
587586
assert!(variants.is_consecutive());
588587
}
589588

590589
#[test]
591590
fn test_new_too_large_rejected() {
592-
// 129 variants exceeds i8::MAX + 1 = 128.
593-
let names: Vec<String> = (0..129).map(|i| format!("v{i}")).collect();
594-
let dtypes: Vec<DType> = (0..129)
591+
let names: Vec<String> = (0..256).map(|i| format!("v{i}")).collect();
592+
let dtypes: Vec<DType> = (0..256)
595593
.map(|_| DType::Primitive(PType::I32, Nullability::NonNullable))
596594
.collect();
597595
let names: FieldNames = names.into_iter().collect();
@@ -601,7 +599,7 @@ mod tests {
601599
result
602600
.unwrap_err()
603601
.to_string()
604-
.contains("at most 128 consecutive variants")
602+
.contains("at most 255 consecutive variants")
605603
);
606604
}
607605

@@ -610,7 +608,7 @@ mod tests {
610608
let v = UnionVariants::empty();
611609
assert!(v.is_empty());
612610
assert_eq!(v.len(), 0);
613-
assert_eq!(v.type_ids(), &[] as &[i8]);
611+
assert_eq!(v.type_ids(), &[] as &[u8]);
614612
assert!(v.is_consecutive());
615613
}
616614
}

‎vortex-flatbuffers/flatbuffers/vortex-dtype/dtype.fbs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ table Variant {
7070
table Union {
7171
names: [string];
7272
dtypes: [DType];
73-
type_ids: [byte]; // length must equal dtypes.len()
73+
type_ids: [ubyte]; // length must equal dtypes.len()
7474
}
7575

7676
union Type {

‎vortex-flatbuffers/src/generated/dtype.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1513,11 +1513,11 @@ impl<'a> Union<'a> {
15131513
unsafe { self._tab.get::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'a, ::flatbuffers::ForwardsUOffset<DType>>>>(Union::VT_DTYPES, None)}
15141514
}
15151515
#[inline]
1516-
pub fn type_ids(&self) -> Option<::flatbuffers::Vector<'a, i8>> {
1516+
pub fn type_ids(&self) -> Option<::flatbuffers::Vector<'a, u8>> {
15171517
// Safety:
15181518
// Created from valid Table for this object
15191519
// which contains a valid value in this slot
1520-
unsafe { self._tab.get::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'a, i8>>>(Union::VT_TYPE_IDS, None)}
1520+
unsafe { self._tab.get::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'a, u8>>>(Union::VT_TYPE_IDS, None)}
15211521
}
15221522
}
15231523

@@ -1529,15 +1529,15 @@ impl ::flatbuffers::Verifiable for Union<'_> {
15291529
v.visit_table(pos)?
15301530
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset<&'_ str>>>>("names", Self::VT_NAMES, false)?
15311531
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset<DType>>>>("dtypes", Self::VT_DTYPES, false)?
1532-
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, i8>>>("type_ids", Self::VT_TYPE_IDS, false)?
1532+
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, u8>>>("type_ids", Self::VT_TYPE_IDS, false)?
15331533
.finish();
15341534
Ok(())
15351535
}
15361536
}
15371537
pub struct UnionArgs<'a> {
15381538
pub names: Option<::flatbuffers::WIPOffset<::flatbuffers::Vector<'a, ::flatbuffers::ForwardsUOffset<&'a str>>>>,
15391539
pub dtypes: Option<::flatbuffers::WIPOffset<::flatbuffers::Vector<'a, ::flatbuffers::ForwardsUOffset<DType<'a>>>>>,
1540-
pub type_ids: Option<::flatbuffers::WIPOffset<::flatbuffers::Vector<'a, i8>>>,
1540+
pub type_ids: Option<::flatbuffers::WIPOffset<::flatbuffers::Vector<'a, u8>>>,
15411541
}
15421542
impl<'a> Default for UnionArgs<'a> {
15431543
#[inline]
@@ -1564,7 +1564,7 @@ impl<'a: 'b, 'b, A: ::flatbuffers::Allocator + 'a> UnionBuilder<'a, 'b, A> {
15641564
self.fbb_.push_slot_always::<::flatbuffers::WIPOffset<_>>(Union::VT_DTYPES, dtypes);
15651565
}
15661566
#[inline]
1567-
pub fn add_type_ids(&mut self, type_ids: ::flatbuffers::WIPOffset<::flatbuffers::Vector<'b , i8>>) {
1567+
pub fn add_type_ids(&mut self, type_ids: ::flatbuffers::WIPOffset<::flatbuffers::Vector<'b , u8>>) {
15681568
self.fbb_.push_slot_always::<::flatbuffers::WIPOffset<_>>(Union::VT_TYPE_IDS, type_ids);
15691569
}
15701570
#[inline]

‎vortex-proto/proto/dtype.proto‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ message Variant {
7777
message Union {
7878
repeated string names = 1;
7979
repeated DType dtypes = 2;
80-
repeated int32 type_ids = 3; // length must equal dtypes.len(); each value must fit in int8
80+
repeated int32 type_ids = 3; // length must equal dtypes.len(); each value must fit in uint8
8181
}
8282

8383
message DType {

0 commit comments

Comments
 (0)