Skip to content

Commit acbb54f

Browse files
committed
better slots fix
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
1 parent 367e666 commit acbb54f

17 files changed

Lines changed: 125 additions & 82 deletions

File tree

vortex-array/src/aggregate_fn/fns/all_non_distinct/map.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ use vortex_error::VortexResult;
55

66
use super::list::check_list_identical;
77
use crate::ExecutionCtx;
8+
use crate::arrays::ListView;
89
use crate::arrays::MapArray;
910
use crate::arrays::map::MapArrayExt;
11+
use crate::arrays::map::MapArraySlotsExt;
1012

1113
pub(super) fn check_map_identical(
1214
lhs: &MapArray,
@@ -18,8 +20,8 @@ pub(super) fn check_map_identical(
1820
}
1921

2022
check_list_identical(
21-
&lhs.entries().into_owned(),
22-
&rhs.entries().into_owned(),
23+
&lhs.entries().as_::<ListView>().into_owned(),
24+
&rhs.entries().as_::<ListView>().into_owned(),
2325
ctx,
2426
)
2527
}

vortex-array/src/aggregate_fn/fns/is_constant/map.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@ use vortex_error::VortexResult;
55

66
use super::list::check_listview_constant;
77
use crate::ExecutionCtx;
8+
use crate::arrays::ListView;
89
use crate::arrays::MapArray;
9-
use crate::arrays::map::MapArrayExt;
10+
use crate::arrays::map::MapArraySlotsExt;
1011

1112
pub(super) fn check_map_constant(map: &MapArray, ctx: &mut ExecutionCtx) -> VortexResult<bool> {
12-
check_listview_constant(&map.entries().into_owned(), ctx)
13+
check_listview_constant(&map.entries().as_::<ListView>().into_owned(), ctx)
1314
}

vortex-array/src/aggregate_fn/fns/uncompressed_size_in_bytes/mod.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ use crate::aggregate_fn::EmptyOptions;
4545
use crate::array::ArrayView;
4646
use crate::arrays::Constant;
4747
use crate::arrays::ConstantArray;
48-
use crate::arrays::map::MapArrayExt;
48+
use crate::arrays::ListView;
49+
use crate::arrays::map::MapArraySlotsExt;
4950
use crate::arrays::varbinview::BinaryView;
5051
use crate::dtype::DType;
5152
use crate::dtype::DecimalType;
@@ -200,9 +201,10 @@ pub(crate) fn canonical_uncompressed_size_in_bytes(
200201
Canonical::Decimal(array) => decimal_uncompressed_size_in_bytes(array, ctx),
201202
Canonical::VarBinView(array) => varbinview_uncompressed_size_in_bytes(array, ctx),
202203
Canonical::List(array) => list_view_uncompressed_size_in_bytes(array, ctx),
203-
Canonical::Map(array) => {
204-
list_view_uncompressed_size_in_bytes(&array.entries().into_owned(), ctx)
205-
}
204+
Canonical::Map(array) => list_view_uncompressed_size_in_bytes(
205+
&array.entries().as_::<ListView>().into_owned(),
206+
ctx,
207+
),
206208
Canonical::FixedSizeList(array) => fixed_size_list_uncompressed_size_in_bytes(array, ctx),
207209
Canonical::Struct(array) => struct_uncompressed_size_in_bytes(array, ctx),
208210
Canonical::Union(array) => union_uncompressed_size_in_bytes(array, ctx),

vortex-array/src/arrays/map/array.rs

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,20 @@ use std::fmt::Formatter;
66
use std::hash::Hasher;
77
use std::sync::Arc;
88

9-
use smallvec::smallvec;
109
use vortex_error::VortexExpect;
1110
use vortex_error::VortexResult;
1211
use vortex_error::vortex_ensure;
1312

1413
use crate::ArrayEq;
1514
use crate::ArrayHash;
1615
use crate::ArrayRef;
16+
use crate::ArraySlots;
1717
use crate::EqMode;
1818
use crate::IntoArray;
1919
use crate::array::Array;
2020
use crate::array::ArrayParts;
21-
use crate::array::ArrayView;
2221
use crate::array::TypedArrayRef;
22+
use crate::array_slots;
2323
use crate::arrays::ListView;
2424
use crate::arrays::ListViewArray;
2525
use crate::arrays::listview::ListViewArrayExt;
@@ -28,10 +28,12 @@ use crate::dtype::DType;
2828
use crate::dtype::MapDType;
2929
use crate::validity::Validity;
3030

31-
/// The one child slot holding a [`ListViewArray`] of map entries.
32-
pub(super) const ENTRIES_SLOT: usize = 0;
33-
pub(super) const NUM_SLOTS: usize = 1;
34-
pub(super) const SLOT_NAMES: [&str; NUM_SLOTS] = ["entries"];
31+
#[array_slots(Map)]
32+
pub struct MapSlots {
33+
/// The list-view storage of non-null `{key, value}` entry structs.
34+
#[slot(0)]
35+
pub entries: ArrayRef,
36+
}
3537

3638
/// Encoding-specific metadata for [`crate::arrays::MapArray`].
3739
///
@@ -56,6 +58,12 @@ impl ArrayHash for MapData {
5658
fn array_hash<H: Hasher>(&self, _state: &mut H, _accuracy: EqMode) {}
5759
}
5860

61+
impl MapData {
62+
pub(crate) fn make_slots(entries: ArrayRef) -> ArraySlots {
63+
MapSlots { entries }.into_slots()
64+
}
65+
}
66+
5967
/// The logical and physical inputs used to construct a [`crate::arrays::MapArray`].
6068
pub struct MapDataParts {
6169
/// The key/value type and sortedness assertion for the map.
@@ -65,28 +73,20 @@ pub struct MapDataParts {
6573
}
6674

6775
/// Accessors for the canonical map representation.
68-
pub trait MapArrayExt: TypedArrayRef<Map> {
69-
/// Returns the list-view storage of map entry structs.
70-
fn entries(&self) -> ArrayView<'_, ListView> {
71-
self.as_ref().slots()[ENTRIES_SLOT]
72-
.as_ref()
73-
.vortex_expect("MapArray entries slot")
74-
.as_::<ListView>()
75-
}
76-
76+
pub trait MapArrayExt: MapArraySlotsExt {
7777
/// Returns the entry structs for one map row.
7878
fn entries_at(&self, index: usize) -> VortexResult<ArrayRef> {
79-
self.entries().list_elements_at(index)
79+
self.entries().as_::<ListView>().list_elements_at(index)
8080
}
8181

8282
/// Returns the number of entries in one map row.
8383
fn entry_count_at(&self, index: usize) -> usize {
84-
self.entries().size_at(index)
84+
self.entries().as_::<ListView>().size_at(index)
8585
}
8686

8787
/// Returns the outer map validity delegated from the entries list-view.
8888
fn map_validity(&self) -> Validity {
89-
self.entries().listview_validity()
89+
self.entries().as_::<ListView>().listview_validity()
9090
}
9191

9292
/// Returns this map's key/value type information.
@@ -125,8 +125,8 @@ impl Array<Map> {
125125
let nullability = entries.nullability();
126126
let dtype = DType::Map(map_dtype, nullability);
127127
let len = entries.len();
128-
let parts = ArrayParts::new(Map, dtype, len, MapData)
129-
.with_slots(smallvec![Some(entries.into_array())]);
128+
let slots = MapData::make_slots(entries.into_array());
129+
let parts = ArrayParts::new(Map, dtype, len, MapData).with_slots(slots);
130130
Self::try_from_parts(parts)
131131
}
132132

@@ -141,8 +141,8 @@ impl Array<Map> {
141141
let nullability = entries.nullability();
142142
let dtype = DType::Map(map_dtype, nullability);
143143
let len = entries.len();
144-
let parts = ArrayParts::new(Map, dtype, len, MapData)
145-
.with_slots(smallvec![Some(entries.into_array())]);
144+
let slots = MapData::make_slots(entries.into_array());
145+
let parts = ArrayParts::new(Map, dtype, len, MapData).with_slots(slots);
146146
unsafe { Self::from_parts_unchecked(parts) }
147147
}
148148

@@ -153,7 +153,7 @@ impl Array<Map> {
153153
.as_map_opt()
154154
.vortex_expect("MapArray requires a map dtype")
155155
.clone();
156-
let entries = self.entries().into_owned();
156+
let entries = self.entries().clone().downcast::<ListView>();
157157
MapDataParts { map_dtype, entries }
158158
}
159159
}

vortex-array/src/arrays/map/compute/cast.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use crate::array::ArrayView;
1111
use crate::arrays::ListView;
1212
use crate::arrays::map::Map;
1313
use crate::arrays::map::MapArrayExt;
14+
use crate::arrays::map::MapArraySlotsExt;
1415
use crate::arrays::map::compute::rebuild_map_from_array;
1516
use crate::dtype::DType;
1617
use crate::dtype::MapDType;
@@ -47,7 +48,10 @@ impl CastReduce for Map {
4748
return Ok(None);
4849
};
4950

50-
let Some(entries) = <ListView as CastReduce>::cast(array.entries(), &target_entries_dtype)?
51+
let Some(entries) = <ListView as CastReduce>::cast(
52+
array.entries().as_::<ListView>(),
53+
&target_entries_dtype,
54+
)?
5155
else {
5256
return Ok(None);
5357
};
@@ -67,8 +71,11 @@ impl CastKernel for Map {
6771
return Ok(None);
6872
};
6973

70-
let Some(entries) =
71-
<ListView as CastKernel>::cast(array.entries(), &target_entries_dtype, ctx)?
74+
let Some(entries) = <ListView as CastKernel>::cast(
75+
array.entries().as_::<ListView>(),
76+
&target_entries_dtype,
77+
ctx,
78+
)?
7279
else {
7380
return Ok(None);
7481
};

vortex-array/src/arrays/map/compute/filter.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,22 @@ use vortex_error::VortexResult;
55
use vortex_mask::Mask;
66

77
use crate::ArrayRef;
8+
use crate::IntoArray;
89
use crate::array::ArrayView;
10+
use crate::arrays::ListView;
911
use crate::arrays::ListViewArray;
12+
use crate::arrays::MapArray;
1013
use crate::arrays::filter::FilterKernel;
1114
use crate::arrays::filter::FilterReduce;
1215
use crate::arrays::listview::ListViewArraySlotsExt;
1316
use crate::arrays::map::Map;
1417
use crate::arrays::map::MapArrayExt;
15-
use crate::arrays::map::compute::rebuild_map;
18+
use crate::arrays::map::MapArraySlotsExt;
1619
use crate::executor::ExecutionCtx;
1720

1821
impl FilterReduce for Map {
1922
fn filter(array: ArrayView<'_, Self>, mask: &Mask) -> VortexResult<Option<ArrayRef>> {
20-
let entries = array.entries();
23+
let entries = array.entries().as_::<ListView>();
2124

2225
// SAFETY: filtering row metadata keeps offsets and sizes paired, preserves the original
2326
// elements, and filters validity to the same output length. The zero-copy-to-list flag is
@@ -31,7 +34,11 @@ impl FilterReduce for Map {
3134
)
3235
};
3336

34-
rebuild_map(array.map_dtype().clone(), filtered_entries).map(Some)
37+
{
38+
let map_dtype = array.map_dtype().clone();
39+
MapArray::try_new(map_dtype, filtered_entries).map(IntoArray::into_array)
40+
}
41+
.map(Some)
3542
}
3643
}
3744

vortex-array/src/arrays/map/compute/mask.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,17 @@ use crate::array::ArrayView;
88
use crate::arrays::ListView;
99
use crate::arrays::map::Map;
1010
use crate::arrays::map::MapArrayExt;
11+
use crate::arrays::map::MapArraySlotsExt;
1112
use crate::arrays::map::compute::rebuild_map_from_array;
1213
use crate::executor::ExecutionCtx;
1314
use crate::scalar_fn::fns::mask::MaskKernel;
1415
use crate::scalar_fn::fns::mask::MaskReduce;
1516

1617
impl MaskReduce for Map {
1718
fn mask(array: ArrayView<'_, Self>, mask: &ArrayRef) -> VortexResult<Option<ArrayRef>> {
18-
let Some(entries) = <ListView as MaskReduce>::mask(array.entries(), mask)? else {
19+
let Some(entries) =
20+
<ListView as MaskReduce>::mask(array.entries().as_::<ListView>(), mask)?
21+
else {
1922
return Ok(None);
2023
};
2124

vortex-array/src/arrays/map/compute/mod.rs

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,16 @@ use vortex_error::vortex_err;
1616
use crate::ArrayRef;
1717
use crate::IntoArray;
1818
use crate::arrays::ListView;
19-
use crate::arrays::ListViewArray;
2019
use crate::arrays::MapArray;
2120
use crate::dtype::MapDType;
2221

23-
fn rebuild_map(map_dtype: MapDType, entries: ListViewArray) -> VortexResult<ArrayRef> {
24-
MapArray::try_new(map_dtype, entries).map(IntoArray::into_array)
25-
}
26-
2722
fn rebuild_map_from_array(map_dtype: MapDType, entries: ArrayRef) -> VortexResult<ArrayRef> {
28-
let entries = entries
29-
.as_opt::<ListView>()
30-
.ok_or_else(|| {
31-
vortex_err!(
32-
"Map entries operation expected vortex.listview/ListView, got {}",
33-
entries.encoding_id()
34-
)
35-
})?
36-
.into_owned();
37-
rebuild_map(map_dtype, entries)
23+
let map_entries = entries.try_downcast::<ListView>().map_err(|arr| {
24+
vortex_err!(
25+
"Map entries operation expected vortex.listview/ListView, got {}",
26+
arr.encoding_id()
27+
)
28+
})?;
29+
30+
MapArray::try_new(map_dtype, map_entries).map(IntoArray::into_array)
3831
}

vortex-array/src/arrays/map/compute/slice.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,17 @@ use crate::array::ArrayView;
1010
use crate::arrays::ListView;
1111
use crate::arrays::map::Map;
1212
use crate::arrays::map::MapArrayExt;
13+
use crate::arrays::map::MapArraySlotsExt;
1314
use crate::arrays::map::compute::rebuild_map_from_array;
1415
use crate::arrays::slice::SliceKernel;
1516
use crate::arrays::slice::SliceReduce;
1617
use crate::executor::ExecutionCtx;
1718

1819
impl SliceReduce for Map {
1920
fn slice(array: ArrayView<'_, Self>, range: Range<usize>) -> VortexResult<Option<ArrayRef>> {
20-
let Some(sliced_entries) = <ListView as SliceReduce>::slice(array.entries(), range)? else {
21+
let Some(sliced_entries) =
22+
<ListView as SliceReduce>::slice(array.entries().as_::<ListView>(), range)?
23+
else {
2124
return Ok(None);
2225
};
2326

vortex-array/src/arrays/map/compute/take.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,15 @@ use crate::arrays::dict::TakeExecute;
1010
use crate::arrays::dict::TakeReduce;
1111
use crate::arrays::map::Map;
1212
use crate::arrays::map::MapArrayExt;
13+
use crate::arrays::map::MapArraySlotsExt;
1314
use crate::arrays::map::compute::rebuild_map_from_array;
1415
use crate::executor::ExecutionCtx;
1516

1617
impl TakeReduce for Map {
1718
fn take(array: ArrayView<'_, Self>, indices: &ArrayRef) -> VortexResult<Option<ArrayRef>> {
18-
let Some(entries) = <ListView as TakeReduce>::take(array.entries(), indices)? else {
19+
let Some(entries) =
20+
<ListView as TakeReduce>::take(array.entries().as_::<ListView>(), indices)?
21+
else {
1922
return Ok(None);
2023
};
2124

@@ -29,7 +32,9 @@ impl TakeExecute for Map {
2932
indices: &ArrayRef,
3033
ctx: &mut ExecutionCtx,
3134
) -> VortexResult<Option<ArrayRef>> {
32-
let Some(entries) = <ListView as TakeExecute>::take(array.entries(), indices, ctx)? else {
35+
let Some(entries) =
36+
<ListView as TakeExecute>::take(array.entries().as_::<ListView>(), indices, ctx)?
37+
else {
3338
return Ok(None);
3439
};
3540

0 commit comments

Comments
 (0)