Skip to content

Commit 633c52d

Browse files
authored
Return an empty array from fill_null on an empty input (#10627)
## Summary Closes #10626. - `FillNull::execute` now returns an empty canonical array of the result dtype (the input dtype with the fill value's nullability) when the input is empty, before it asks for a scalar fill. Compare and the boolean operators already return early for empty inputs in the same way. - New test `empty_input_ignores_non_constant_fill` runs `FillNull` on an empty input with an empty, non-constant fill and checks the result is empty and non-nullable. - New test `repeated_fill_over_empty_chunk` builds a chunked array with an empty chunk, fills nulls twice and checks the values. This is the shape the fuzzer found. --------- Signed-off-by: Vincent <1729065730@qq.com>
1 parent 1671cb3 commit 633c52d

1 file changed

Lines changed: 35 additions & 3 deletions

File tree

  • vortex-array/src/scalar_fn/fns/fill_null

‎vortex-array/src/scalar_fn/fns/fill_null/mod.rs‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use vortex_session::registry::CachedId;
1313

1414
use crate::AnyColumnar;
1515
use crate::ArrayRef;
16+
use crate::Canonical;
1617
use crate::CanonicalView;
1718
use crate::ColumnarView;
1819
use crate::ExecutionCtx;
@@ -131,9 +132,18 @@ impl ScalarFnVTable for FillNull {
131132
fill_value
132133
};
133134

134-
let fill_scalar = fill_value
135-
.as_constant()
136-
.ok_or_else(|| vortex_err!("fill_null fill_value must be a constant/scalar"))?;
135+
let Some(fill_scalar) = fill_value.as_constant() else {
136+
// Slicing a constant fill to an empty range yields an empty canonical array rather
137+
// than a constant. An empty input has no nulls to fill, so there is nothing to do.
138+
vortex_ensure!(
139+
input.is_empty(),
140+
"fill_null fill_value must be a constant/scalar"
141+
);
142+
let dtype = input
143+
.dtype()
144+
.with_nullability(fill_value.dtype().nullability());
145+
return Ok(Canonical::empty(&dtype).into_array());
146+
};
137147

138148
vortex_ensure!(
139149
!fill_scalar.is_null(),
@@ -403,6 +413,28 @@ mod tests {
403413
Ok(())
404414
}
405415

416+
/// Filling a chunked array slices the constant fill per chunk, and an empty slice is a
417+
/// canonical array rather than a constant. Stacked fills over an empty chunk must still run.
418+
#[test]
419+
fn repeated_fill_over_empty_chunk() -> VortexResult<()> {
420+
let mut ctx = array_session().create_execution_ctx();
421+
let input = ChunkedArray::try_new(
422+
vec![
423+
PrimitiveArray::from_option_iter(Vec::<Option<u16>>::new()).into_array(),
424+
PrimitiveArray::from_option_iter([Some(7u16), None]).into_array(),
425+
],
426+
DType::Primitive(PType::U16, Nullability::Nullable),
427+
)?
428+
.into_array();
429+
430+
let result = input
431+
.fill_null(Scalar::primitive(1u16, Nullability::Nullable))?
432+
.fill_null(Scalar::primitive(2u16, Nullability::NonNullable))?;
433+
434+
assert_arrays_eq!(result, PrimitiveArray::from_iter([7u16, 1]), &mut ctx);
435+
Ok(())
436+
}
437+
406438
#[test]
407439
fn evaluate_struct_field() {
408440
let mut ctx = array_session().create_execution_ctx();

0 commit comments

Comments
 (0)