Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions datafusion/functions-nested/src/array_has.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ fn array_has_inner_for_array(haystack: &ArrayRef, needle: &ArrayRef) -> Result<A
array_has_dispatch_for_array(haystack, needle)
}

#[derive(Copy, Clone)]
enum ArrayWrapper<'a> {
FixedSizeList(&'a arrow::array::FixedSizeListArray),
List(&'a arrow::array::GenericListArray<i32>),
Expand Down Expand Up @@ -317,8 +318,8 @@ impl<'a> ArrayWrapper<'a> {
}
}

fn array_has_dispatch_for_array(
haystack: ArrayWrapper<'_>,
fn array_has_dispatch_for_array<'a>(
haystack: ArrayWrapper<'a>,
needle: &ArrayRef,
) -> Result<ArrayRef> {
let mut boolean_builder = BooleanArray::builder(haystack.len());
Expand Down Expand Up @@ -390,8 +391,8 @@ fn array_has_all_inner(args: &[ArrayRef]) -> Result<ArrayRef> {

// General row comparison for array_has_all and array_has_any
fn general_array_has_for_all_and_any<'a>(
haystack: &ArrayWrapper<'a>,
needle: &ArrayWrapper<'a>,
haystack: ArrayWrapper<'a>,
needle: ArrayWrapper<'a>,
comparison_type: ComparisonType,
) -> Result<ArrayRef> {
let mut boolean_builder = BooleanArray::builder(haystack.len());
Expand All @@ -402,8 +403,8 @@ fn general_array_has_for_all_and_any<'a>(
let arr_values = converter.convert_columns(&[arr])?;
let sub_arr_values = converter.convert_columns(&[sub_arr])?;
boolean_builder.append_value(general_array_has_all_and_any_kernel(
arr_values,
sub_arr_values,
&arr_values,
&sub_arr_values,
comparison_type,
));
} else {
Expand All @@ -416,8 +417,8 @@ fn general_array_has_for_all_and_any<'a>(

// String comparison for array_has_all and array_has_any
fn array_has_all_and_any_string_internal<'a>(
haystack: &ArrayWrapper<'a>,
needle: &ArrayWrapper<'a>,
haystack: ArrayWrapper<'a>,
needle: ArrayWrapper<'a>,
comparison_type: ComparisonType,
) -> Result<ArrayRef> {
let mut boolean_builder = BooleanArray::builder(haystack.len());
Expand All @@ -427,8 +428,8 @@ fn array_has_all_and_any_string_internal<'a>(
let haystack_array = string_array_to_vec(&arr);
let needle_array = string_array_to_vec(&sub_arr);
boolean_builder.append_value(array_has_string_kernel(
haystack_array,
needle_array,
&haystack_array,
&needle_array,
comparison_type,
));
}
Expand All @@ -442,8 +443,8 @@ fn array_has_all_and_any_string_internal<'a>(
}

fn array_has_all_and_any_dispatch<'a>(
haystack: &ArrayWrapper<'a>,
needle: &ArrayWrapper<'a>,
haystack: ArrayWrapper<'a>,
needle: ArrayWrapper<'a>,
comparison_type: ComparisonType,
) -> Result<ArrayRef> {
if needle.values().is_empty() {
Expand All @@ -468,7 +469,7 @@ fn array_has_all_and_any_inner(
) -> Result<ArrayRef> {
let haystack: ArrayWrapper = args[0].as_ref().try_into()?;
let needle: ArrayWrapper = args[1].as_ref().try_into()?;
array_has_all_and_any_dispatch(&haystack, &needle, comparison_type)
array_has_all_and_any_dispatch(haystack, needle, comparison_type)
}

fn array_has_any_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
Expand Down Expand Up @@ -633,8 +634,8 @@ enum ComparisonType {
}

fn array_has_string_kernel(
haystack: Vec<Option<&str>>,
needle: Vec<Option<&str>>,
haystack: &[Option<&str>],
needle: &[Option<&str>],
comparison_type: ComparisonType,
) -> bool {
match comparison_type {
Expand All @@ -650,8 +651,8 @@ fn array_has_string_kernel(
}

fn general_array_has_all_and_any_kernel(
haystack_rows: Rows,
needle_rows: Rows,
haystack_rows: &Rows,
needle_rows: &Rows,
comparison_type: ComparisonType,
) -> bool {
match comparison_type {
Expand Down
12 changes: 6 additions & 6 deletions datafusion/functions-nested/src/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
let (inner_field, inner_offsets, inner_values, _) =
as_list_array(&values)?.clone().into_parts();
let offsets =
get_offsets_for_flatten::<i32, i32>(inner_offsets, offsets);
get_offsets_for_flatten::<i32, i32>(inner_offsets, &offsets);
let flattened_array = GenericListArray::<i32>::new(
inner_field,
offsets,
Expand All @@ -159,7 +159,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
let (inner_field, inner_offsets, inner_values, _) =
as_large_list_array(&values)?.clone().into_parts();
let offsets =
get_offsets_for_flatten::<i64, i32>(inner_offsets, offsets);
get_offsets_for_flatten::<i64, i32>(inner_offsets, &offsets);
let flattened_array = GenericListArray::<i64>::new(
inner_field,
offsets,
Expand All @@ -180,7 +180,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
List(_) => {
let (inner_field, inner_offsets, inner_values, _) =
as_list_array(&values)?.clone().into_parts();
let offsets = get_large_offsets_for_flatten(inner_offsets, offsets);
let offsets = get_large_offsets_for_flatten(inner_offsets, &offsets);
let flattened_array = GenericListArray::<i64>::new(
inner_field,
offsets,
Expand All @@ -194,7 +194,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
let (inner_field, inner_offsets, inner_values, _) =
as_large_list_array(&values)?.clone().into_parts();
let offsets =
get_offsets_for_flatten::<i64, i64>(inner_offsets, offsets);
get_offsets_for_flatten::<i64, i64>(inner_offsets, &offsets);
let flattened_array = GenericListArray::<i64>::new(
inner_field,
offsets,
Expand All @@ -217,7 +217,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
// Create new offsets that are equivalent to `flatten` the array.
fn get_offsets_for_flatten<O: OffsetSizeTrait, P: OffsetSizeTrait>(
inner_offsets: OffsetBuffer<O>,
outer_offsets: OffsetBuffer<P>,
outer_offsets: &OffsetBuffer<P>,
) -> OffsetBuffer<O> {
let buffer = inner_offsets.into_inner();
let offsets: Vec<O> = outer_offsets
Expand All @@ -230,7 +230,7 @@ fn get_offsets_for_flatten<O: OffsetSizeTrait, P: OffsetSizeTrait>(
// Create new large offsets that are equivalent to `flatten` the array.
fn get_large_offsets_for_flatten<O: OffsetSizeTrait, P: OffsetSizeTrait>(
inner_offsets: OffsetBuffer<O>,
outer_offsets: OffsetBuffer<P>,
outer_offsets: &OffsetBuffer<P>,
) -> OffsetBuffer<i64> {
let buffer = inner_offsets.into_inner();
let offsets: Vec<i64> = outer_offsets
Expand Down
3 changes: 3 additions & 0 deletions datafusion/functions-nested/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
// Make sure fast / cheap clones on Arc are explicit:
// https://github.com/apache/datafusion/issues/11143
#![deny(clippy::clone_on_ref_ptr)]
// https://github.com/apache/datafusion/issues/18503
#![deny(clippy::needless_pass_by_value)]
#![cfg_attr(test, allow(clippy::needless_pass_by_value))]

//! Nested type Functions for [DataFusion].
//!
Expand Down
56 changes: 28 additions & 28 deletions datafusion/functions-nested/src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ fn make_map_batch(args: &[ColumnarValue]) -> Result<ColumnarValue> {

let values = get_first_array_ref(values_arg)?;

make_map_batch_internal(keys, values, can_evaluate_to_const, keys_arg.data_type())
make_map_batch_internal(&keys, &values, can_evaluate_to_const, &keys_arg.data_type())
}

/// Validates that map keys are non-null and unique.
Expand Down Expand Up @@ -126,10 +126,10 @@ fn get_first_array_ref(columnar_value: &ColumnarValue) -> Result<ArrayRef> {
}

fn make_map_batch_internal(
keys: ArrayRef,
values: ArrayRef,
keys: &ArrayRef,
values: &ArrayRef,
can_evaluate_to_const: bool,
data_type: DataType,
data_type: &DataType,
) -> Result<ColumnarValue> {
if keys.len() != values.len() {
return exec_err!("map requires key and value lists to have the same length");
Expand Down Expand Up @@ -159,8 +159,8 @@ fn make_map_batch_internal(
let mut entry_offsets_buffer = VecDeque::new();
entry_offsets_buffer.push_back(0);

entry_struct_buffer.push_back((Arc::clone(&key_field), Arc::clone(&keys)));
entry_struct_buffer.push_back((Arc::clone(&value_field), Arc::clone(&values)));
entry_struct_buffer.push_back((Arc::clone(&key_field), Arc::clone(keys)));
entry_struct_buffer.push_back((Arc::clone(&value_field), Arc::clone(values)));
entry_offsets_buffer.push_back(keys.len() as u32);

let entry_struct: Vec<(Arc<Field>, ArrayRef)> = entry_struct_buffer.into();
Expand Down Expand Up @@ -368,8 +368,8 @@ fn get_element_type(data_type: &DataType) -> Result<&DataType> {
/// +-----------+ +-----------+
/// ```text
fn make_map_array_internal<O: OffsetSizeTrait>(
keys: ArrayRef,
values: ArrayRef,
keys: &ArrayRef,
values: &ArrayRef,
) -> Result<ColumnarValue> {
// Save original data types and array length before list_to_arrays transforms them
let keys_data_type = keys.data_type().clone();
Expand All @@ -380,14 +380,14 @@ fn make_map_array_internal<O: OffsetSizeTrait>(
// This tells us which MAP values are NULL (not which keys within maps are null)
let nulls_bitmap = keys.nulls().cloned();

let keys = list_to_arrays::<O>(&keys);
let values = list_to_arrays::<O>(&values);
let keys = list_to_arrays::<O>(keys);
let values = list_to_arrays::<O>(values);

build_map_array(
keys,
values,
keys_data_type,
values_data_type,
&keys,
&values,
&keys_data_type,
&values_data_type,
original_len,
nulls_bitmap,
)
Expand All @@ -396,8 +396,8 @@ fn make_map_array_internal<O: OffsetSizeTrait>(
/// Helper function specifically for FixedSizeList inputs
/// Similar to make_map_array_internal but uses fixed_size_list_to_arrays instead of list_to_arrays
fn make_map_array_from_fixed_size_list(
keys: ArrayRef,
values: ArrayRef,
keys: &ArrayRef,
values: &ArrayRef,
) -> Result<ColumnarValue> {
// Save original data types and array length
let keys_data_type = keys.data_type().clone();
Expand All @@ -407,25 +407,25 @@ fn make_map_array_from_fixed_size_list(
// Save the nulls bitmap from the original keys array
let nulls_bitmap = keys.nulls().cloned();

let keys = fixed_size_list_to_arrays(&keys);
let values = fixed_size_list_to_arrays(&values);
let keys = fixed_size_list_to_arrays(keys);
let values = fixed_size_list_to_arrays(values);

build_map_array(
keys,
values,
keys_data_type,
values_data_type,
&keys,
&values,
&keys_data_type,
&values_data_type,
original_len,
nulls_bitmap,
)
}

/// Common logic to build a MapArray from decomposed list arrays
fn build_map_array(
keys: Vec<ArrayRef>,
values: Vec<ArrayRef>,
keys_data_type: DataType,
values_data_type: DataType,
keys: &[ArrayRef],
values: &[ArrayRef],
keys_data_type: &DataType,
values_data_type: &DataType,
original_len: usize,
nulls_bitmap: Option<arrow::buffer::NullBuffer>,
) -> Result<ColumnarValue> {
Expand Down Expand Up @@ -470,8 +470,8 @@ fn build_map_array(
let (flattened_keys, flattened_values) = if key_array_vec.is_empty() {
// All maps are NULL - create empty arrays
// We need to infer the data type from the original keys/values arrays
let key_type = get_element_type(&keys_data_type)?;
let value_type = get_element_type(&values_data_type)?;
let key_type = get_element_type(keys_data_type)?;
let value_type = get_element_type(values_data_type)?;

(
arrow::array::new_empty_array(key_type),
Expand Down
4 changes: 2 additions & 2 deletions datafusion/functions-nested/src/position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,13 @@ fn general_position_dispatch<O: OffsetSizeTrait>(args: &[ArrayRef]) -> Result<Ar
);
}

generic_position::<O>(list_array, element_array, arr_from)
generic_position::<O>(list_array, element_array, &arr_from)
}

fn generic_position<OffsetSize: OffsetSizeTrait>(
list_array: &GenericListArray<OffsetSize>,
element_array: &ArrayRef,
arr_from: Vec<i64>, // 0-indexed
arr_from: &[i64], // 0-indexed
) -> Result<ArrayRef> {
let mut data = Vec::with_capacity(list_array.len());

Expand Down
10 changes: 5 additions & 5 deletions datafusion/functions-nested/src/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,29 +289,29 @@ pub fn array_remove_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
let [array, element] = take_function_args("array_remove", args)?;

let arr_n = vec![1; array.len()];
array_remove_internal(array, element, arr_n)
array_remove_internal(array, element, &arr_n)
}

/// Array_remove_n SQL function
pub fn array_remove_n_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
let [array, element, max] = take_function_args("array_remove_n", args)?;

let arr_n = as_int64_array(max)?.values().to_vec();
array_remove_internal(array, element, arr_n)
array_remove_internal(array, element, &arr_n)
}

/// Array_remove_all SQL function
pub fn array_remove_all_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
let [array, element] = take_function_args("array_remove_all", args)?;

let arr_n = vec![i64::MAX; array.len()];
array_remove_internal(array, element, arr_n)
array_remove_internal(array, element, &arr_n)
}

fn array_remove_internal(
array: &ArrayRef,
element_array: &ArrayRef,
arr_n: Vec<i64>,
arr_n: &[i64],
) -> Result<ArrayRef> {
match array.data_type() {
DataType::List(_) => {
Expand Down Expand Up @@ -348,7 +348,7 @@ fn array_remove_internal(
fn general_remove<OffsetSize: OffsetSizeTrait>(
list_array: &GenericListArray<OffsetSize>,
element_array: &ArrayRef,
arr_n: Vec<i64>,
arr_n: &[i64],
) -> Result<ArrayRef> {
let data_type = list_array.value_type();
let mut new_values = vec![];
Expand Down
Loading