Skip to content

Commit be75eca

Browse files
joseph-isaacsclaude
andcommitted
feat[array]: use owned matchers at call sites
Replace borrow-then-clone patterns with owned matchers: - filter/masked vtable: use into_::<AnyCanonical>() instead of Canonical::from(child.as_::<AnyCanonical>()) - canonical/columnar Executable: use try_into_matched instead of as_opt + Canonical::from - dict take_canonical: accept owned Canonical directly Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent bacecaf commit be75eca

6 files changed

Lines changed: 11 additions & 25 deletions

File tree

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use vortex_error::VortexExpect;
77
use vortex_error::VortexResult;
88

99
use crate::Canonical;
10-
use crate::CanonicalView;
1110
use crate::ExecutionCtx;
1211
use crate::IntoArray;
1312
use crate::arrays::Bool;
@@ -37,11 +36,10 @@ use crate::arrays::variant::VariantArrayExt;
3736
/// This is the core operation for dictionary decoding - it expands the dictionary
3837
/// by looking up each code in the values array.
3938
pub(crate) fn take_canonical(
40-
values: CanonicalView,
39+
values: Canonical,
4140
codes: &PrimitiveArray,
4241
ctx: &mut ExecutionCtx,
4342
) -> VortexResult<Canonical> {
44-
let values = Canonical::from(values);
4543
Ok(match values {
4644
Canonical::Null(a) => Canonical::Null(take_null(&a, codes)),
4745
Canonical::Bool(a) => Canonical::Bool(take_bool(&a, codes, ctx)?),

vortex-array/src/arrays/dict/vtable/mod.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -190,11 +190,10 @@ impl VTable for Dict {
190190

191191
let DictParts { values, codes, .. } = array.into_parts();
192192

193-
Ok(ExecutionResult::done(take_canonical(
194-
values.as_::<AnyCanonical>(),
195-
&codes.downcast::<Primitive>(),
196-
ctx,
197-
)?))
193+
let codes = codes.downcast::<Primitive>();
194+
let values = values.into_::<AnyCanonical>();
195+
196+
Ok(ExecutionResult::done(take_canonical(values, &codes, ctx)?))
198197
}
199198

200199
fn reduce_parent(

vortex-array/src/arrays/filter/vtable.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use crate::AnyCanonical;
1717
use crate::ArrayEq;
1818
use crate::ArrayHash;
1919
use crate::ArrayRef;
20-
use crate::Canonical;
2120
use crate::IntoArray;
2221
use crate::Precision;
2322
use crate::array::Array;
@@ -155,8 +154,7 @@ impl VTable for Filter {
155154

156155
// We rely on the optimization pass that runs prior to this execution for filter pushdown,
157156
// so now we can just execute the filter without worrying.
158-
// TODO(joe): fix the ownership of AnyCanonical
159-
let child = Canonical::from(array.child().as_::<AnyCanonical>());
157+
let child = array.child().clone().into_::<AnyCanonical>();
160158
Ok(ExecutionResult::done(
161159
execute_filter(child, &mask_values).into_array(),
162160
))

vortex-array/src/arrays/masked/vtable/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use crate::AnyCanonical;
1919
use crate::ArrayEq;
2020
use crate::ArrayHash;
2121
use crate::ArrayRef;
22-
use crate::Canonical;
2322
use crate::IntoArray;
2423
use crate::LEGACY_SESSION;
2524
use crate::Precision;
@@ -178,7 +177,7 @@ impl VTable for Masked {
178177
// While we could manually convert the dtype, `mask_validity_canonical` is already O(1) for
179178
// `AllTrue` masks (no data copying), so there's no benefit.
180179

181-
let child = Canonical::from(array.child().as_::<AnyCanonical>());
180+
let child = array.child().clone().into_::<AnyCanonical>();
182181
Ok(ExecutionResult::done(
183182
mask_validity_canonical(child, validity, ctx)?.into_array(),
184183
))

vortex-array/src/canonical.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,8 +543,7 @@ impl Executable for Canonical {
543543
fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult<Self> {
544544
let result = array.execute_until::<AnyCanonical>(ctx)?;
545545
Ok(result
546-
.as_opt::<AnyCanonical>()
547-
.map(Canonical::from)
546+
.try_into_matched::<AnyCanonical>()
548547
.vortex_expect("execute_until::<AnyCanonical> must return a canonical array"))
549548
}
550549
}

vortex-array/src/columnar.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,9 @@ impl IntoArray for Columnar {
7272
impl Executable for Columnar {
7373
fn execute(array: ArrayRef, ctx: &mut ExecutionCtx) -> VortexResult<Self> {
7474
let result = array.execute_until::<AnyColumnar>(ctx)?;
75-
if let Some(constant) = result.as_opt::<Constant>() {
76-
Ok(Columnar::Constant(constant.into_owned()))
77-
} else {
78-
Ok(Columnar::Canonical(
79-
result
80-
.as_opt::<AnyCanonical>()
81-
.map(Canonical::from)
82-
.vortex_expect("execute_until::<AnyColumnar> must return a columnar array"),
83-
))
84-
}
75+
Ok(result
76+
.try_into_matched::<AnyColumnar>()
77+
.vortex_expect("execute_until::<AnyColumnar> must return a columnar array"))
8578
}
8679
}
8780

0 commit comments

Comments
 (0)