Skip to content

Commit bc69d12

Browse files
committed
fixes
Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent bf38b29 commit bc69d12

2 files changed

Lines changed: 62 additions & 157 deletions

File tree

vortex-array/src/arrays/dict/execute.rs

Lines changed: 51 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33

44
//! Execution logic for DictArray - takes from values using codes (indices).
55
6-
use vortex_error::VortexExpect;
76
use vortex_error::VortexResult;
7+
use vortex_error::{VortexExpect, vortex_bail};
88

9+
use crate::ArrayView;
910
use crate::Canonical;
1011
use crate::CanonicalView;
1112
use crate::ExecutionCtx;
12-
use crate::IntoArray;
1313
use crate::arrays::Bool;
1414
use crate::arrays::BoolArray;
1515
use crate::arrays::Decimal;
@@ -20,6 +20,7 @@ use crate::arrays::FixedSizeList;
2020
use crate::arrays::FixedSizeListArray;
2121
use crate::arrays::ListView;
2222
use crate::arrays::ListViewArray;
23+
use crate::arrays::Null;
2324
use crate::arrays::NullArray;
2425
use crate::arrays::Primitive;
2526
use crate::arrays::PrimitiveArray;
@@ -38,148 +39,138 @@ use crate::arrays::variant::VariantArraySlotsExt;
3839
/// by looking up each code in the values array.
3940
pub(crate) fn take_canonical(
4041
values: CanonicalView,
41-
codes: &PrimitiveArray,
42+
codes: ArrayView<'_, Primitive>,
4243
ctx: &mut ExecutionCtx,
4344
) -> VortexResult<Canonical> {
44-
let values = Canonical::from(values);
4545
Ok(match values {
46-
Canonical::Null(a) => Canonical::Null(take_null(&a, codes)),
47-
Canonical::Bool(a) => Canonical::Bool(take_bool(&a, codes, ctx)?),
48-
Canonical::Primitive(a) => Canonical::Primitive(take_primitive(&a, codes, ctx)),
49-
Canonical::Decimal(a) => Canonical::Decimal(take_decimal(&a, codes, ctx)),
50-
Canonical::VarBinView(a) => Canonical::VarBinView(take_varbinview(&a, codes, ctx)),
51-
Canonical::List(a) => Canonical::List(take_listview(&a, codes, ctx)),
52-
Canonical::Map(_) => vortex_error::vortex_bail!("Map arrays don't support take"),
53-
Canonical::FixedSizeList(a) => {
54-
Canonical::FixedSizeList(take_fixed_size_list(&a, codes, ctx))
46+
CanonicalView::Null(a) => Canonical::Null(take_null(a, codes)),
47+
CanonicalView::Bool(a) => Canonical::Bool(take_bool(a, codes, ctx)?),
48+
CanonicalView::Primitive(a) => Canonical::Primitive(take_primitive(a, codes, ctx)),
49+
CanonicalView::Decimal(a) => Canonical::Decimal(take_decimal(a, codes, ctx)),
50+
CanonicalView::VarBinView(a) => Canonical::VarBinView(take_varbinview(a, codes, ctx)),
51+
CanonicalView::List(a) => Canonical::List(take_listview(a, codes, ctx)),
52+
Canonical::Map(_) => vortex_bail!("Map arrays don't support take"),
53+
CanonicalView::FixedSizeList(a) => {
54+
Canonical::FixedSizeList(take_fixed_size_list(a, codes, ctx))
5555
}
56-
Canonical::Struct(a) => Canonical::Struct(take_struct(&a, codes)),
57-
Canonical::Union(_) => {
56+
CanonicalView::Struct(a) => Canonical::Struct(take_struct(a, codes)),
57+
CanonicalView::Union(_) => {
5858
todo!(
5959
"TODO(connor)[Union]: implement dictionary execution after Union take supports \
6060
nullable indices and outer null propagation"
6161
)
6262
}
63-
Canonical::Extension(a) => Canonical::Extension(take_extension(&a, codes, ctx)),
64-
Canonical::Variant(a) => {
65-
let indices = codes.clone().into_array();
63+
CanonicalView::Extension(a) => Canonical::Extension(take_extension(a, codes, ctx)),
64+
CanonicalView::Variant(a) => {
65+
let indices = codes.array().clone();
6666
let taken_core_storage = a.core_storage().take(indices.clone())?;
6767
let taken_shredded = a
6868
.shredded()
69-
.map(|shredded| shredded.take(indices.clone()))
69+
.map(|shredded| shredded.take(indices))
7070
.transpose()?;
7171
Canonical::Variant(VariantArray::try_new(taken_core_storage, taken_shredded)?)
7272
}
7373
})
7474
}
7575

7676
/// Take for NullArray is trivial - just create a new NullArray with the new length.
77-
fn take_null(_array: &NullArray, codes: &PrimitiveArray) -> NullArray {
77+
fn take_null(_array: ArrayView<'_, Null>, codes: ArrayView<'_, Primitive>) -> NullArray {
7878
NullArray::new(codes.len())
7979
}
8080

81-
// TODO(joe): use dict_bool_take
8281
fn take_bool(
83-
array: &BoolArray,
84-
codes: &PrimitiveArray,
82+
array: ArrayView<'_, Bool>,
83+
codes: ArrayView<'_, Primitive>,
8584
ctx: &mut ExecutionCtx,
8685
) -> VortexResult<BoolArray> {
87-
let codes_ref = codes.clone().into_array();
88-
let array = array.as_view();
89-
Ok(<Bool as TakeExecute>::take(array, &codes_ref, ctx)?
86+
let codes_ref = codes.array();
87+
Ok(<Bool as TakeExecute>::take(array, codes_ref, ctx)?
9088
.vortex_expect("take bool should not return None")
9189
.as_::<Bool>()
9290
.into_owned())
9391
}
9492

9593
fn take_primitive(
96-
array: &PrimitiveArray,
97-
codes: &PrimitiveArray,
94+
array: ArrayView<'_, Primitive>,
95+
codes: ArrayView<'_, Primitive>,
9896
ctx: &mut ExecutionCtx,
9997
) -> PrimitiveArray {
100-
let codes_ref = codes.clone().into_array();
101-
let array = array.as_view();
102-
<Primitive as TakeExecute>::take(array, &codes_ref, ctx)
98+
let codes_ref = codes.array();
99+
<Primitive as TakeExecute>::take(array, codes_ref, ctx)
103100
.vortex_expect("take primitive array")
104101
.vortex_expect("take primitive should not return None")
105102
.as_::<Primitive>()
106103
.into_owned()
107104
}
108105

109106
fn take_decimal(
110-
array: &DecimalArray,
111-
codes: &PrimitiveArray,
107+
array: ArrayView<'_, Decimal>,
108+
codes: ArrayView<'_, Primitive>,
112109
ctx: &mut ExecutionCtx,
113110
) -> DecimalArray {
114-
let codes_ref = codes.clone().into_array();
115-
let array = array.as_view();
116-
<Decimal as TakeExecute>::take(array, &codes_ref, ctx)
111+
let codes_ref = codes.array();
112+
<Decimal as TakeExecute>::take(array, codes_ref, ctx)
117113
.vortex_expect("take decimal array")
118114
.vortex_expect("take decimal should not return None")
119115
.as_::<Decimal>()
120116
.into_owned()
121117
}
122118

123119
fn take_varbinview(
124-
array: &VarBinViewArray,
125-
codes: &PrimitiveArray,
120+
array: ArrayView<'_, VarBinView>,
121+
codes: ArrayView<'_, Primitive>,
126122
ctx: &mut ExecutionCtx,
127123
) -> VarBinViewArray {
128-
let codes_ref = codes.clone().into_array();
129-
let array = array.as_view();
130-
<VarBinView as TakeExecute>::take(array, &codes_ref, ctx)
124+
let codes_ref = codes.array();
125+
<VarBinView as TakeExecute>::take(array, codes_ref, ctx)
131126
.vortex_expect("take varbinview array")
132127
.vortex_expect("take varbinview should not return None")
133128
.as_::<VarBinView>()
134129
.into_owned()
135130
}
136131

137132
fn take_listview(
138-
array: &ListViewArray,
139-
codes: &PrimitiveArray,
133+
array: ArrayView<'_, ListView>,
134+
codes: ArrayView<'_, Primitive>,
140135
ctx: &mut ExecutionCtx,
141136
) -> ListViewArray {
142-
let codes_ref = codes.clone().into_array();
143-
let array = array.as_view();
144-
<ListView as TakeExecute>::take(array, &codes_ref, ctx)
137+
let codes_ref = codes.array();
138+
<ListView as TakeExecute>::take(array, codes_ref, ctx)
145139
.vortex_expect("take listview execute")
146140
.vortex_expect("ListView TakeExecute should not return None")
147141
.as_::<ListView>()
148142
.into_owned()
149143
}
150144

151145
fn take_fixed_size_list(
152-
array: &FixedSizeListArray,
153-
codes: &PrimitiveArray,
146+
array: ArrayView<'_, FixedSizeList>,
147+
codes: ArrayView<'_, Primitive>,
154148
ctx: &mut ExecutionCtx,
155149
) -> FixedSizeListArray {
156-
let codes_ref = codes.clone().into_array();
157-
let array = array.as_view();
158-
<FixedSizeList as TakeExecute>::take(array, &codes_ref, ctx)
150+
let codes_ref = codes.array();
151+
<FixedSizeList as TakeExecute>::take(array, codes_ref, ctx)
159152
.vortex_expect("take fixed size list array")
160153
.vortex_expect("take fixed size list should not return None")
161154
.as_::<FixedSizeList>()
162155
.into_owned()
163156
}
164157

165-
fn take_struct(array: &StructArray, codes: &PrimitiveArray) -> StructArray {
166-
let codes_ref = codes.clone().into_array();
167-
let array = array.as_view();
168-
<Struct as TakeReduce>::take(array, &codes_ref)
158+
fn take_struct(array: ArrayView<'_, Struct>, codes: ArrayView<'_, Primitive>) -> StructArray {
159+
let codes_ref = codes.array();
160+
<Struct as TakeReduce>::take(array, codes_ref)
169161
.vortex_expect("take struct array")
170162
.vortex_expect("take struct should not return None")
171163
.as_::<Struct>()
172164
.into_owned()
173165
}
174166

175167
fn take_extension(
176-
array: &ExtensionArray,
177-
codes: &PrimitiveArray,
168+
array: ArrayView<'_, Extension>,
169+
codes: ArrayView<'_, Primitive>,
178170
ctx: &mut ExecutionCtx,
179171
) -> ExtensionArray {
180-
let codes_ref = codes.clone().into_array();
181-
let array = array.as_view();
182-
<Extension as TakeExecute>::take(array, &codes_ref, ctx)
172+
let codes_ref = codes.array();
173+
<Extension as TakeExecute>::take(array, codes_ref, ctx)
183174
.vortex_expect("take extension storage")
184175
.vortex_expect("take extension should not return None")
185176
.as_::<Extension>()

0 commit comments

Comments
 (0)