Skip to content

Commit beed4f0

Browse files
authored
perf: Optimize NULL handling in array_slice (#21482)
## Which issue does this PR close? - Closes #21481. ## Rationale for this change `array_slice` does a per-row NULL checking (in four different arrays!). It would be faster to take the union of the four input NULL buffers via `NullBuffer::union`. Benchmarks (Arm64): ``` - List(Int64), array args: 60.98ms -> 58.35ms (-4.3%) - List(Int64), array args, no stride: 28.19ms -> 25.92ms (-8.0%) - List(Int64), scalar args, no stride: 57.07ms -> 55.56ms (-2.6%) - List(Int64), scalar args, stride=-2: 99.44ms -> 96.05ms (-3.4%) - List(Int64), scalar args, stride=-1: 155.23ms -> 155.30ms (+0.0%) - List(Int64), scalar args, stride=1: 58.50ms -> 55.91ms (-4.4%) - List(Int64), scalar args, stride=2: 151.45ms -> 146.83ms (-3.1%) - ListView(Int64), array args: 56.19ms -> 52.86ms (-5.9%) - ListView(Int64), array args, no stride: 28.53ms -> 24.35ms (-14.7%) - ListView(Int64), scalar args, no stride: 58.65ms -> 58.34ms (-0.5%) - ListView(Int64), scalar args, stride=-2: 93.85ms -> 91.59ms (-2.4%) - ListView(Int64), scalar args, stride=-1: 149.68ms -> 149.06ms (-0.4%) - ListView(Int64), scalar args, stride=1: 59.53ms -> 58.90ms (-1.1%) - ListView(Int64), scalar args, stride=2: 143.07ms -> 139.55ms (-2.5%) ``` ## What changes are included in this PR? ## Are these changes tested? Yes. ## Are there any user-facing changes? No.
1 parent 42d9835 commit beed4f0

File tree

1 file changed

+27
-22
lines changed

1 file changed

+27
-22
lines changed

datafusion/functions-nested/src/extract.rs

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@
1919
2020
use arrow::array::{
2121
Array, ArrayRef, Capacities, GenericListArray, GenericListViewArray, Int64Array,
22-
MutableArrayData, NullArray, NullBufferBuilder, OffsetSizeTrait,
22+
MutableArrayData, NullArray, OffsetSizeTrait,
2323
};
24-
use arrow::buffer::{OffsetBuffer, ScalarBuffer};
24+
use arrow::buffer::{NullBuffer, OffsetBuffer, ScalarBuffer};
2525
use arrow::datatypes::DataType;
2626
use arrow::datatypes::{
2727
DataType::{FixedSizeList, LargeList, LargeListView, List, ListView, Null},
@@ -595,6 +595,23 @@ where
595595
}
596596
}
597597

598+
/// Combine null bitmaps from all slice inputs into a single mask.
599+
fn combine_input_nulls(
600+
array: &dyn Array,
601+
from_array: &Int64Array,
602+
to_array: &Int64Array,
603+
stride: Option<&Int64Array>,
604+
) -> Option<NullBuffer> {
605+
[
606+
array.nulls(),
607+
from_array.nulls(),
608+
to_array.nulls(),
609+
stride.and_then(|s| s.nulls()),
610+
]
611+
.into_iter()
612+
.fold(None, |acc, nulls| NullBuffer::union(acc.as_ref(), nulls))
613+
}
614+
598615
fn general_array_slice<O: OffsetSizeTrait>(
599616
array: &GenericListArray<O>,
600617
from_array: &Int64Array,
@@ -615,25 +632,19 @@ where
615632
// The rule `adjusted_from_index` and `adjusted_to_index` follows the rule of array_slice in duckdb.
616633

617634
let mut offsets = vec![O::usize_as(0)];
618-
let mut null_builder = NullBufferBuilder::new(array.len());
635+
636+
let nulls = combine_input_nulls(array, from_array, to_array, stride);
619637

620638
for (row_index, offset_window) in array.offsets().windows(2).enumerate() {
621639
let start = offset_window[0];
622640
let end = offset_window[1];
623641
let len = end - start;
624642

625-
// If any input is null, return null.
626-
if array.is_null(row_index)
627-
|| from_array.is_null(row_index)
628-
|| to_array.is_null(row_index)
629-
|| stride.is_some_and(|s| s.is_null(row_index))
630-
{
643+
if nulls.as_ref().is_some_and(|n| n.is_null(row_index)) {
631644
mutable.extend_nulls(1);
632645
offsets.push(offsets[row_index] + O::usize_as(1));
633-
null_builder.append_null();
634646
continue;
635647
}
636-
null_builder.append_non_null();
637648

638649
// Empty arrays always return an empty array.
639650
if len == O::usize_as(0) {
@@ -676,7 +687,7 @@ where
676687
Arc::new(Field::new_list_field(array.value_type(), true)),
677688
OffsetBuffer::<O>::new(offsets.into()),
678689
arrow::array::make_array(data),
679-
null_builder.finish(),
690+
nulls,
680691
)?))
681692
}
682693

@@ -707,21 +718,15 @@ where
707718
let mut offsets = Vec::with_capacity(array.len());
708719
let mut sizes = Vec::with_capacity(array.len());
709720
let mut current_offset = O::usize_as(0);
710-
let mut null_builder = NullBufferBuilder::new(array.len());
721+
722+
let nulls = combine_input_nulls(array, from_array, to_array, stride);
711723

712724
for row_index in 0..array.len() {
713-
// Propagate NULL semantics: any NULL input yields a NULL output slot.
714-
if array.is_null(row_index)
715-
|| from_array.is_null(row_index)
716-
|| to_array.is_null(row_index)
717-
|| stride.is_some_and(|s| s.is_null(row_index))
718-
{
719-
null_builder.append_null();
725+
if nulls.as_ref().is_some_and(|n| n.is_null(row_index)) {
720726
offsets.push(current_offset);
721727
sizes.push(O::usize_as(0));
722728
continue;
723729
}
724-
null_builder.append_non_null();
725730

726731
let len = array.value_size(row_index);
727732

@@ -777,7 +782,7 @@ where
777782
ScalarBuffer::from(offsets),
778783
ScalarBuffer::from(sizes),
779784
arrow::array::make_array(data),
780-
null_builder.finish(),
785+
nulls,
781786
)?))
782787
}
783788

0 commit comments

Comments
 (0)