Skip to content

Commit ef964a6

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

2 files changed

Lines changed: 60 additions & 155 deletions

File tree

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

Lines changed: 49 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
use vortex_error::VortexExpect;
77
use vortex_error::VortexResult;
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,147 +39,137 @@ 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::FixedSizeList(a) => {
53-
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+
CanonicalView::FixedSizeList(a) => {
53+
Canonical::FixedSizeList(take_fixed_size_list(a, codes, ctx))
5454
}
55-
Canonical::Struct(a) => Canonical::Struct(take_struct(&a, codes)),
56-
Canonical::Union(_) => {
55+
CanonicalView::Struct(a) => Canonical::Struct(take_struct(a, codes)),
56+
CanonicalView::Union(_) => {
5757
todo!(
5858
"TODO(connor)[Union]: implement dictionary execution after Union take supports \
5959
nullable indices and outer null propagation"
6060
)
6161
}
62-
Canonical::Extension(a) => Canonical::Extension(take_extension(&a, codes, ctx)),
63-
Canonical::Variant(a) => {
64-
let indices = codes.clone().into_array();
62+
CanonicalView::Extension(a) => Canonical::Extension(take_extension(a, codes, ctx)),
63+
CanonicalView::Variant(a) => {
64+
let indices = codes.array().clone();
6565
let taken_core_storage = a.core_storage().take(indices.clone())?;
6666
let taken_shredded = a
6767
.shredded()
68-
.map(|shredded| shredded.take(indices.clone()))
68+
.map(|shredded| shredded.take(indices))
6969
.transpose()?;
7070
Canonical::Variant(VariantArray::try_new(taken_core_storage, taken_shredded)?)
7171
}
7272
})
7373
}
7474

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

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

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

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

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

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

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

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

174166
fn take_extension(
175-
array: &ExtensionArray,
176-
codes: &PrimitiveArray,
167+
array: ArrayView<'_, Extension>,
168+
codes: ArrayView<'_, Primitive>,
177169
ctx: &mut ExecutionCtx,
178170
) -> ExtensionArray {
179-
let codes_ref = codes.clone().into_array();
180-
let array = array.as_view();
181-
<Extension as TakeExecute>::take(array, &codes_ref, ctx)
171+
let codes_ref = codes.array();
172+
<Extension as TakeExecute>::take(array, codes_ref, ctx)
182173
.vortex_expect("take extension storage")
183174
.vortex_expect("take extension should not return None")
184175
.as_::<Extension>()

0 commit comments

Comments
 (0)