chore[array]: fill null remove and dep fill_null(array, scalar) (#6437)

## Does this PR closes an open issue or discussion?

<!--
This helps us keep track of fixed issues and changes.
-->

- Closes #.

## What changes are included in this PR?

Cleaned & deprecated up the `fill_null` compute function

<!--
What changes are included here, if an issue or discussion are attached,
there's no need to duplicate the details.
-->

## What is the rationale for this change?

<!--
Why do you propose this change, and why did you choose this approach.

This helps reviewers and other readers understand changes, creates a
shared understanding of the issue and codebase,
and improves their ability to work with this change and offer better
suggestions.
-->

## How is this change tested?

<!--
Changes should be tested, we expect changes to fit in one of the
following categories:
1. Verifying existing behavior is maintained.
2. For serialization related changes - Compatibility should be
maintained or explicitly broken.
3. For new behavior and functionality, this helps us maintaining that
desired behavior in the future.
-->

## Are there any user-facing changes?

<!--
Does the change affect users in what of the following ways:
1. Breaks public APIs in some way.
2. Changes the underlying behavior of one of the integrations.
3. Should some documentation be changed to reflect this change?

In the case some public API is changed in a breaking way, make sure to
add the appropriate label.
-->

`#[deprecated(note = "use array.fill_null(scalar) via ArrayBuiltins
instead")]`

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>

Author: joseph-isaacs

encodings/alp/src/alp_rd/compute/take.rs

@@ -6,7 +6,7 @@ use vortex_array::ArrayRef;
 use vortex_array::ExecutionCtx;
 use vortex_array::IntoArray;
 use vortex_array::arrays::TakeExecute;
-use vortex_array::compute::fill_null;
+use vortex_array::builtins::ArrayBuiltins;
 use vortex_error::VortexResult;
 use vortex_scalar::Scalar;
 
@@ -33,10 +33,10 @@ impl TakeExecute for ALPRDVTable {
                 p.cast_values(&values_dtype)
             })
             .transpose()?;
-        let right_parts = fill_null(
-            &array.right_parts().take(indices.to_array())?,
-            &Scalar::zero_value(array.right_parts().dtype()),
-        )?;
+        let right_parts = array
+            .right_parts()
+            .take(indices.to_array())?
+            .fill_null(Scalar::zero_value(array.right_parts().dtype()))?;
 
         Ok(Some(
             ALPRDArray::try_new(

encodings/datetime-parts/src/compute/take.rs

@@ -7,7 +7,7 @@ use vortex_array::ExecutionCtx;
 use vortex_array::IntoArray;
 use vortex_array::ToCanonical;
 use vortex_array::arrays::TakeExecute;
-use vortex_array::compute::fill_null;
+use vortex_array::builtins::ArrayBuiltins;
 use vortex_array::expr::stats::Stat;
 use vortex_array::expr::stats::StatsProvider;
 use vortex_dtype::Nullability;
@@ -68,7 +68,7 @@ fn take_datetime_parts(array: &DateTimePartsArray, indices: &dyn Array) -> Vorte
         .map(|s| s.into_inner())
         .unwrap_or_else(|| Scalar::primitive(0i64, Nullability::NonNullable))
         .cast(array.seconds().dtype())?;
-    let taken_seconds = fill_null(taken_seconds.as_ref(), &seconds_fill)?;
+    let taken_seconds = taken_seconds.fill_null(seconds_fill)?;
 
     let subseconds_fill = array
         .subseconds()
@@ -77,7 +77,7 @@ fn take_datetime_parts(array: &DateTimePartsArray, indices: &dyn Array) -> Vorte
         .map(|s| s.into_inner())
         .unwrap_or_else(|| Scalar::primitive(0i64, Nullability::NonNullable))
         .cast(array.subseconds().dtype())?;
-    let taken_subseconds = fill_null(taken_subseconds.as_ref(), &subseconds_fill)?;
+    let taken_subseconds = taken_subseconds.fill_null(subseconds_fill)?;
 
     Ok(
         DateTimePartsArray::try_new(dtype, taken_days, taken_seconds, taken_subseconds)?

encodings/fsst/src/compute/mod.rs

@@ -11,7 +11,7 @@ use vortex_array::ExecutionCtx;
 use vortex_array::IntoArray;
 use vortex_array::arrays::TakeExecute;
 use vortex_array::arrays::VarBinVTable;
-use vortex_array::compute::fill_null;
+use vortex_array::builtins::ArrayBuiltins;
 use vortex_error::VortexExpect;
 use vortex_error::VortexResult;
 use vortex_error::vortex_err;
@@ -38,10 +38,12 @@ impl TakeExecute for FSSTVTable {
                     .vortex_expect("cannot fail")
                     .try_into::<VarBinVTable>()
                     .map_err(|_| vortex_err!("take for codes must return varbin array"))?,
-                fill_null(
-                    &array.uncompressed_lengths().take(indices.to_array())?,
-                    &Scalar::zero_value(&array.uncompressed_lengths_dtype().clone()),
-                )?,
+                array
+                    .uncompressed_lengths()
+                    .take(indices.to_array())?
+                    .fill_null(Scalar::zero_value(
+                        &array.uncompressed_lengths_dtype().clone(),
+                    ))?,
             )?
             .into_array(),
         ))

encodings/sparse/src/lib.rs

@@ -19,9 +19,9 @@ use vortex_array::ProstMetadata;
 use vortex_array::ToCanonical;
 use vortex_array::arrays::ConstantArray;
 use vortex_array::buffer::BufferHandle;
+use vortex_array::builtins::ArrayBuiltins;
 use vortex_array::compute::Operator;
 use vortex_array::compute::compare;
-use vortex_array::compute::fill_null;
 use vortex_array::compute::filter;
 use vortex_array::compute::sub_scalar;
 use vortex_array::patches::Patches;
@@ -343,12 +343,10 @@ impl SparseArray {
 
         let fill_array = ConstantArray::new(fill.clone(), array.len()).into_array();
         let non_top_mask = Mask::from_buffer(
-            fill_null(
-                &compare(array, &fill_array, Operator::NotEq)?,
-                &Scalar::bool(true, Nullability::NonNullable),
-            )?
-            .to_bool()
-            .to_bit_buffer(),
+            compare(array, &fill_array, Operator::NotEq)?
+                .fill_null(Scalar::bool(true, Nullability::NonNullable))?
+                .to_bool()
+                .to_bit_buffer(),
         );
 
         let non_top_values = filter(array, &non_top_mask)?;

fuzz/src/array/fill_null.rs

@@ -10,7 +10,7 @@ use vortex_array::arrays::ConstantArray;
 use vortex_array::arrays::DecimalArray;
 use vortex_array::arrays::PrimitiveArray;
 use vortex_array::arrays::VarBinViewArray;
-use vortex_array::compute::fill_null;
+use vortex_array::builtins::ArrayBuiltins;
 use vortex_array::validity::Validity;
 use vortex_array::vtable::ValidityHelper;
 use vortex_buffer::Buffer;
@@ -43,7 +43,7 @@ pub fn fill_null_canonical_array(
         Canonical::Struct(_)
         | Canonical::List(_)
         | Canonical::FixedSizeList(_)
-        | Canonical::Extension(_) => fill_null(canonical.as_ref(), fill_value)?,
+        | Canonical::Extension(_) => canonical.into_array().fill_null(fill_value.clone())?,
     })
 }
 

fuzz/src/array/mod.rs

@@ -551,9 +551,9 @@ pub fn compress_array(array: &dyn Array, _strategy: CompressorStrategy) -> Array
 #[allow(clippy::result_large_err)]
 pub fn run_fuzz_action(fuzz_action: FuzzArrayAction) -> crate::error::VortexFuzzResult<bool> {
     use vortex_array::arrays::ConstantArray;
+    use vortex_array::builtins::ArrayBuiltins;
     use vortex_array::compute::cast;
     use vortex_array::compute::compare;
-    use vortex_array::compute::fill_null;
     use vortex_array::compute::mask;
     use vortex_array::compute::min_max;
     use vortex_array::compute::sum;
@@ -636,7 +636,8 @@ pub fn run_fuzz_action(fuzz_action: FuzzArrayAction) -> crate::error::VortexFuzz
                 assert_min_max_eq(&expected.min_max(), &min_max_result, i)?;
             }
             Action::FillNull(fill_value) => {
-                current_array = fill_null(&current_array, &fill_value)
+                current_array = current_array
+                    .fill_null(fill_value.clone())
                     .vortex_expect("fill_null operation should succeed in fuzz test");
                 assert_array_eq(&expected.array(), &current_array, i)?;
             }

vortex-array/src/arrays/bool/compute/fill_null.rs

@@ -47,10 +47,11 @@ mod tests {
     use vortex_buffer::bitbuffer;
     use vortex_dtype::DType;
     use vortex_dtype::Nullability;
+    use vortex_scalar::Scalar;
 
     use crate::arrays::BoolArray;
+    use crate::builtins::ArrayBuiltins;
     use crate::canonical::ToCanonical;
-    use crate::compute::fill_null;
     use crate::validity::Validity;
 
     #[rstest]
@@ -61,7 +62,9 @@ mod tests {
             BitBuffer::from_iter([true, true, false, false]),
             Validity::from_iter([true, false, true, false]),
         );
-        let non_null_array = fill_null(bool_array.as_ref(), &fill_value.into())
+        let non_null_array = bool_array
+            .to_array()
+            .fill_null(Scalar::from(fill_value))
             .unwrap()
             .to_bool();
         assert_eq!(non_null_array.to_bit_buffer(), expected);

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

@@ -18,7 +18,7 @@ use crate::arrays::BoolArray;
 use crate::arrays::BoolVTable;
 use crate::arrays::ConstantArray;
 use crate::arrays::TakeExecute;
-use crate::compute::fill_null;
+use crate::builtins::ArrayBuiltins;
 use crate::executor::ExecutionCtx;
 use crate::vtable::ValidityHelper;
 
@@ -36,7 +36,9 @@ impl TakeExecute for BoolVTable {
                         .into_array(),
                 ));
             }
-            Mask::Values(_) => fill_null(indices, &Scalar::from(0).cast(indices.dtype())?)?,
+            Mask::Values(_) => indices
+                .to_array()
+                .fill_null(Scalar::from(0).cast(indices.dtype())?)?,
         };
         let indices_nulls_zeroed = indices_nulls_zeroed.to_primitive();
         let buffer = match_each_integer_ptype!(indices_nulls_zeroed.ptype(), |I| {

vortex-array/src/arrays/chunked/compute/fill_null.rs

@@ -32,11 +32,12 @@ mod tests {
     use vortex_buffer::BitBuffer;
     use vortex_dtype::DType;
     use vortex_dtype::Nullability;
+    use vortex_scalar::Scalar;
 
     use crate::array::Array;
     use crate::arrays::BoolArray;
     use crate::arrays::ChunkedArray;
-    use crate::compute::fill_null;
+    use crate::builtins::ArrayBuiltins;
     use crate::validity::Validity;
 
     #[test]
@@ -50,7 +51,7 @@ mod tests {
         )
         .unwrap();
 
-        let filled = fill_null(chunked.as_ref(), &false.into()).unwrap();
+        let filled = chunked.to_array().fill_null(Scalar::from(false)).unwrap();
         assert_eq!(*filled.dtype(), DType::Bool(Nullability::NonNullable));
     }
 }

vortex-array/src/arrays/constant/compute/fill_null.rs

@@ -30,15 +30,14 @@ mod test {
     use crate::IntoArray as _;
     use crate::arrays::ConstantArray;
     use crate::arrow::IntoArrowArray as _;
-    use crate::compute::fill_null;
+    use crate::builtins::ArrayBuiltins;
 
     #[test]
     fn test_null() {
-        let actual = fill_null(
-            &ConstantArray::new(Scalar::null_native::<i32>(), 3).into_array(),
-            &Scalar::from(1),
-        )
-        .unwrap();
+        let actual = ConstantArray::new(Scalar::null_native::<i32>(), 3)
+            .into_array()
+            .fill_null(Scalar::from(1))
+            .unwrap();
         let expected = ConstantArray::new(Scalar::from(1), 3).into_array();
 
         assert!(!actual.dtype().is_nullable());
@@ -56,11 +55,10 @@ mod test {
 
     #[test]
     fn test_non_null() {
-        let actual = fill_null(
-            &ConstantArray::new(Scalar::from(Some(1)), 3).into_array(),
-            &Scalar::from(1),
-        )
-        .unwrap();
+        let actual = ConstantArray::new(Scalar::from(Some(1)), 3)
+            .into_array()
+            .fill_null(Scalar::from(1))
+            .unwrap();
         let expected = ConstantArray::new(Scalar::from(1), 3).into_array();
 
         assert!(!actual.dtype().is_nullable());
@@ -78,11 +76,10 @@ mod test {
 
     #[test]
     fn test_non_nullable_with_nullable() {
-        let actual = fill_null(
-            &ConstantArray::new(Scalar::from(1), 3).into_array(),
-            &Scalar::from(Some(1)),
-        )
-        .unwrap();
+        let actual = ConstantArray::new(Scalar::from(1), 3)
+            .into_array()
+            .fill_null(Scalar::from(Some(1)))
+            .unwrap();
         let expected = ConstantArray::new(Scalar::from(1), 3).into_array();
 
         assert!(!Scalar::from(1).dtype().is_nullable());