refactor(array): drop the Narrow zip kernel for the if_false branch

ZipExecuteAdaptor dispatches only on the if_true branch, so the second
kernel needed its own execute-parent impl and a branch enum. A Narrow in
the if_false branch now takes the default path, as every other encoding
does.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>

Author: connortsui20

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

@@ -30,7 +30,7 @@ pub(crate) mod numeric;
 pub(crate) mod rules;
 mod slice;
 mod take;
-pub(crate) mod zip;
+mod zip;
 
 use vortex_error::VortexResult;
 

vortex-array/src/arrays/narrow/compute/zip.rs

@@ -1,105 +1,52 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
-use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
 
 use crate::ArrayRef;
 use crate::ExecutionCtx;
 use crate::array::ArrayView;
 use crate::arrays::Narrow;
-use crate::arrays::ScalarFn;
 use crate::arrays::narrow::NarrowArraySlotsExt;
 use crate::arrays::narrow::compute::cast_keeping_nullability;
 use crate::arrays::narrow::compute::ptype_holding_both_values;
 use crate::arrays::narrow::compute::ptype_holding_values_and_scalar;
 use crate::arrays::narrow::compute::wrap_values;
-use crate::arrays::scalar_fn::ExactScalarFn;
-use crate::arrays::scalar_fn::ScalarFnArrayExt;
-use crate::arrays::scalar_fn::ScalarFnArrayView;
 use crate::builtins::ArrayBuiltins;
-use crate::kernel::ExecuteParentKernel;
-use crate::scalar_fn::fns::zip::Zip;
 use crate::scalar_fn::fns::zip::ZipKernel;
 
+/// Zips the values of a [`Narrow`] in the `if_true` branch with the `if_false` branch, when that is
+/// another `Narrow` or a constant, at the type that holds both. Any other `if_false` declines.
+///
+/// [`ZipExecuteAdaptor`](crate::scalar_fn::fns::zip::ZipExecuteAdaptor) runs this kernel only for
+/// the `if_true` branch, so a `Narrow` in the `if_false` branch takes the default path.
 impl ZipKernel for Narrow {
     fn zip(
         array: ArrayView<'_, Self>,
         if_false: &ArrayRef,
         mask: &ArrayRef,
         _ctx: &mut ExecutionCtx,
     ) -> VortexResult<Option<ArrayRef>> {
-        zip_values(array, if_false, mask, Branch::IfTrue)
-    }
-}
-
-/// Zips a [`Narrow`] in the `if_false` branch.
-///
-/// [`ZipExecuteAdaptor`] runs [`ZipKernel`] only for the `if_true` branch.
-///
-/// [`ZipExecuteAdaptor`]: crate::scalar_fn::fns::zip::ZipExecuteAdaptor
-#[derive(Debug)]
-pub(crate) struct NarrowZipIfFalseKernel;
-
-impl ExecuteParentKernel<Narrow> for NarrowZipIfFalseKernel {
-    type Parent = ExactScalarFn<Zip>;
-
-    fn execute_parent(
-        &self,
-        array: ArrayView<'_, Narrow>,
-        parent: ScalarFnArrayView<'_, Zip>,
-        child_idx: usize,
-        _ctx: &mut ExecutionCtx,
-    ) -> VortexResult<Option<ArrayRef>> {
-        if child_idx != 1 {
-            return Ok(None);
-        }
-
-        let zip = parent
-            .as_opt::<ScalarFn>()
-            .vortex_expect("ExactScalarFn matcher confirmed ScalarFnArray");
-
-        zip_values(array, zip.get_child(0), zip.get_child(2), Branch::IfFalse)
-    }
-}
-
-/// The branch of a zip that holds the [`Narrow`].
-enum Branch {
-    IfTrue,
-    IfFalse,
-}
-
-/// Zips the values of `array` with the values of `other`, a `Narrow` or a constant, at the type
-/// that holds both. Any other `other` declines.
-fn zip_values(
-    array: ArrayView<'_, Narrow>,
-    other: &ArrayRef,
-    mask: &ArrayRef,
-    branch: Branch,
-) -> VortexResult<Option<ArrayRef>> {
-    let values = array.values();
-    let (other_values, ptype) = if let Some(other) = other.as_opt::<Narrow>() {
-        let Some(ptype) = ptype_holding_both_values(array, other) else {
+        let (if_false_values, ptype) = if let Some(other) = if_false.as_opt::<Narrow>() {
+            let Some(ptype) = ptype_holding_both_values(array, other) else {
+                return Ok(None);
+            };
+            (other.values().clone(), ptype)
+        } else if let Some(constant) = if_false.as_constant() {
+            let Some(ptype) = ptype_holding_values_and_scalar(array, constant)? else {
+                return Ok(None);
+            };
+            (if_false.clone(), ptype)
+        } else {
             return Ok(None);
         };
-        (other.values().clone(), ptype)
-    } else if let Some(constant) = other.as_constant() {
-        let Some(ptype) = ptype_holding_values_and_scalar(array, constant)? else {
-            return Ok(None);
-        };
-        (other.clone(), ptype)
-    } else {
-        return Ok(None);
-    };
 
-    let values = cast_keeping_nullability(values, ptype)?;
-    let other_values = cast_keeping_nullability(&other_values, ptype)?;
+        let if_true_values = cast_keeping_nullability(array.values(), ptype)?;
+        let if_false_values = cast_keeping_nullability(&if_false_values, ptype)?;
 
-    let zipped = match branch {
-        Branch::IfTrue => mask.zip(values, other_values)?,
-        Branch::IfFalse => mask.zip(other_values, values)?,
-    };
-    let dtype = array.dtype().with_nullability(zipped.dtype().nullability());
+        let zipped = mask.zip(if_true_values, if_false_values)?;
+        let dtype = array.dtype().with_nullability(zipped.dtype().nullability());
 
-    wrap_values(zipped, dtype).map(Some)
+        wrap_values(zipped, dtype).map(Some)
+    }
 }

vortex-array/src/arrays/narrow/mod.rs

@@ -40,7 +40,6 @@ use crate::array::VTable;
 use crate::arrays::Dict;
 use crate::arrays::dict::TakeExecuteAdaptor;
 use crate::arrays::narrow::compute::numeric::NarrowNumericKernel;
-use crate::arrays::narrow::compute::zip::NarrowZipIfFalseKernel;
 use crate::optimizer::kernels::ArrayKernelsExt;
 use crate::scalar_fn::ScalarFnVTable;
 use crate::scalar_fn::fns::binary::Binary;
@@ -54,7 +53,6 @@ pub(crate) fn initialize(session: &VortexSession) {
     kernels.register_execute_parent_kernel(Binary.id(), Narrow, NarrowNumericKernel);
     kernels.register_execute_parent_kernel(Dict.id(), Narrow, TakeExecuteAdaptor(Narrow));
     kernels.register_execute_parent_kernel(Zip.id(), Narrow, ZipExecuteAdaptor(Narrow));
-    kernels.register_execute_parent_kernel(Zip.id(), Narrow, NarrowZipIfFalseKernel);
 }
 
 #[cfg(test)]

vortex-array/src/arrays/narrow/tests/compute.rs

@@ -245,18 +245,13 @@ fn test_fill_null(
     Ok(())
 }
 
-/// The zip runs on the values at the type that holds both branches. A constant in the `if_true`
-/// branch is the only input that reaches the `if_false` kernel, because an execution step stops at
-/// the first child kernel that answers. The null mask row takes the `if_false` branch.
+/// The zip runs on the values at the type that holds both branches. The null mask row takes the
+/// `if_false` branch.
 #[rstest]
 #[case::constant_if_false(
     signed().into_array(),
     ConstantArray::new(1_000i64, 5).into_array(),
 )]
-#[case::constant_if_true(
-    ConstantArray::new(1_000i64, 5).into_array(),
-    signed().into_array(),
-)]
 #[case::narrow_both(
     signed().into_array(),
     narrow(buffer![1_000i16, 2, 300, -4, 5], PType::I64).into_array(),