Skip to content

Commit 240e2e3

Browse files
authored
chore: enforce clippy lint needless_pass_by_value to datafusion-functions-nested (#18839)
## Which issue does this PR close? - Closes #18835. - Part of #18503. ## What changes are included in this PR? enforce clippy lint `needless_pass_by_value` to `datafusion-functions-nested` ## Are these changes tested? yes ## Are there any user-facing changes? no
1 parent 1e9c1f4 commit 240e2e3

File tree

9 files changed

+100
-96
lines changed

9 files changed

+100
-96
lines changed

datafusion/functions-nested/src/array_has.rs

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ fn array_has_inner_for_array(haystack: &ArrayRef, needle: &ArrayRef) -> Result<A
239239
array_has_dispatch_for_array(haystack, needle)
240240
}
241241

242+
#[derive(Copy, Clone)]
242243
enum ArrayWrapper<'a> {
243244
FixedSizeList(&'a arrow::array::FixedSizeListArray),
244245
List(&'a arrow::array::GenericListArray<i32>),
@@ -317,8 +318,8 @@ impl<'a> ArrayWrapper<'a> {
317318
}
318319
}
319320

320-
fn array_has_dispatch_for_array(
321-
haystack: ArrayWrapper<'_>,
321+
fn array_has_dispatch_for_array<'a>(
322+
haystack: ArrayWrapper<'a>,
322323
needle: &ArrayRef,
323324
) -> Result<ArrayRef> {
324325
let mut boolean_builder = BooleanArray::builder(haystack.len());
@@ -390,8 +391,8 @@ fn array_has_all_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
390391

391392
// General row comparison for array_has_all and array_has_any
392393
fn general_array_has_for_all_and_any<'a>(
393-
haystack: &ArrayWrapper<'a>,
394-
needle: &ArrayWrapper<'a>,
394+
haystack: ArrayWrapper<'a>,
395+
needle: ArrayWrapper<'a>,
395396
comparison_type: ComparisonType,
396397
) -> Result<ArrayRef> {
397398
let mut boolean_builder = BooleanArray::builder(haystack.len());
@@ -402,8 +403,8 @@ fn general_array_has_for_all_and_any<'a>(
402403
let arr_values = converter.convert_columns(&[arr])?;
403404
let sub_arr_values = converter.convert_columns(&[sub_arr])?;
404405
boolean_builder.append_value(general_array_has_all_and_any_kernel(
405-
arr_values,
406-
sub_arr_values,
406+
&arr_values,
407+
&sub_arr_values,
407408
comparison_type,
408409
));
409410
} else {
@@ -416,8 +417,8 @@ fn general_array_has_for_all_and_any<'a>(
416417

417418
// String comparison for array_has_all and array_has_any
418419
fn array_has_all_and_any_string_internal<'a>(
419-
haystack: &ArrayWrapper<'a>,
420-
needle: &ArrayWrapper<'a>,
420+
haystack: ArrayWrapper<'a>,
421+
needle: ArrayWrapper<'a>,
421422
comparison_type: ComparisonType,
422423
) -> Result<ArrayRef> {
423424
let mut boolean_builder = BooleanArray::builder(haystack.len());
@@ -427,8 +428,8 @@ fn array_has_all_and_any_string_internal<'a>(
427428
let haystack_array = string_array_to_vec(&arr);
428429
let needle_array = string_array_to_vec(&sub_arr);
429430
boolean_builder.append_value(array_has_string_kernel(
430-
haystack_array,
431-
needle_array,
431+
&haystack_array,
432+
&needle_array,
432433
comparison_type,
433434
));
434435
}
@@ -442,8 +443,8 @@ fn array_has_all_and_any_string_internal<'a>(
442443
}
443444

444445
fn array_has_all_and_any_dispatch<'a>(
445-
haystack: &ArrayWrapper<'a>,
446-
needle: &ArrayWrapper<'a>,
446+
haystack: ArrayWrapper<'a>,
447+
needle: ArrayWrapper<'a>,
447448
comparison_type: ComparisonType,
448449
) -> Result<ArrayRef> {
449450
if needle.values().is_empty() {
@@ -468,7 +469,7 @@ fn array_has_all_and_any_inner(
468469
) -> Result<ArrayRef> {
469470
let haystack: ArrayWrapper = args[0].as_ref().try_into()?;
470471
let needle: ArrayWrapper = args[1].as_ref().try_into()?;
471-
array_has_all_and_any_dispatch(&haystack, &needle, comparison_type)
472+
array_has_all_and_any_dispatch(haystack, needle, comparison_type)
472473
}
473474

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

635636
fn array_has_string_kernel(
636-
haystack: Vec<Option<&str>>,
637-
needle: Vec<Option<&str>>,
637+
haystack: &[Option<&str>],
638+
needle: &[Option<&str>],
638639
comparison_type: ComparisonType,
639640
) -> bool {
640641
match comparison_type {
@@ -650,8 +651,8 @@ fn array_has_string_kernel(
650651
}
651652

652653
fn general_array_has_all_and_any_kernel(
653-
haystack_rows: Rows,
654-
needle_rows: Rows,
654+
haystack_rows: &Rows,
655+
needle_rows: &Rows,
655656
comparison_type: ComparisonType,
656657
) -> bool {
657658
match comparison_type {

datafusion/functions-nested/src/flatten.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
145145
let (inner_field, inner_offsets, inner_values, _) =
146146
as_list_array(&values)?.clone().into_parts();
147147
let offsets =
148-
get_offsets_for_flatten::<i32, i32>(inner_offsets, offsets);
148+
get_offsets_for_flatten::<i32, i32>(inner_offsets, &offsets);
149149
let flattened_array = GenericListArray::<i32>::new(
150150
inner_field,
151151
offsets,
@@ -159,7 +159,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
159159
let (inner_field, inner_offsets, inner_values, _) =
160160
as_large_list_array(&values)?.clone().into_parts();
161161
let offsets =
162-
get_offsets_for_flatten::<i64, i32>(inner_offsets, offsets);
162+
get_offsets_for_flatten::<i64, i32>(inner_offsets, &offsets);
163163
let flattened_array = GenericListArray::<i64>::new(
164164
inner_field,
165165
offsets,
@@ -180,7 +180,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
180180
List(_) => {
181181
let (inner_field, inner_offsets, inner_values, _) =
182182
as_list_array(&values)?.clone().into_parts();
183-
let offsets = get_large_offsets_for_flatten(inner_offsets, offsets);
183+
let offsets = get_large_offsets_for_flatten(inner_offsets, &offsets);
184184
let flattened_array = GenericListArray::<i64>::new(
185185
inner_field,
186186
offsets,
@@ -194,7 +194,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
194194
let (inner_field, inner_offsets, inner_values, _) =
195195
as_large_list_array(&values)?.clone().into_parts();
196196
let offsets =
197-
get_offsets_for_flatten::<i64, i64>(inner_offsets, offsets);
197+
get_offsets_for_flatten::<i64, i64>(inner_offsets, &offsets);
198198
let flattened_array = GenericListArray::<i64>::new(
199199
inner_field,
200200
offsets,
@@ -217,7 +217,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
217217
// Create new offsets that are equivalent to `flatten` the array.
218218
fn get_offsets_for_flatten<O: OffsetSizeTrait, P: OffsetSizeTrait>(
219219
inner_offsets: OffsetBuffer<O>,
220-
outer_offsets: OffsetBuffer<P>,
220+
outer_offsets: &OffsetBuffer<P>,
221221
) -> OffsetBuffer<O> {
222222
let buffer = inner_offsets.into_inner();
223223
let offsets: Vec<O> = outer_offsets
@@ -230,7 +230,7 @@ fn get_offsets_for_flatten<O: OffsetSizeTrait, P: OffsetSizeTrait>(
230230
// Create new large offsets that are equivalent to `flatten` the array.
231231
fn get_large_offsets_for_flatten<O: OffsetSizeTrait, P: OffsetSizeTrait>(
232232
inner_offsets: OffsetBuffer<O>,
233-
outer_offsets: OffsetBuffer<P>,
233+
outer_offsets: &OffsetBuffer<P>,
234234
) -> OffsetBuffer<i64> {
235235
let buffer = inner_offsets.into_inner();
236236
let offsets: Vec<i64> = outer_offsets

datafusion/functions-nested/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
// Make sure fast / cheap clones on Arc are explicit:
2424
// https://github.com/apache/datafusion/issues/11143
2525
#![deny(clippy::clone_on_ref_ptr)]
26+
// https://github.com/apache/datafusion/issues/18503
27+
#![deny(clippy::needless_pass_by_value)]
28+
#![cfg_attr(test, allow(clippy::needless_pass_by_value))]
2629

2730
//! Nested type Functions for [DataFusion].
2831
//!

datafusion/functions-nested/src/map.rs

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ fn make_map_batch(args: &[ColumnarValue]) -> Result<ColumnarValue> {
8989

9090
let values = get_first_array_ref(values_arg)?;
9191

92-
make_map_batch_internal(keys, values, can_evaluate_to_const, keys_arg.data_type())
92+
make_map_batch_internal(&keys, &values, can_evaluate_to_const, &keys_arg.data_type())
9393
}
9494

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

128128
fn make_map_batch_internal(
129-
keys: ArrayRef,
130-
values: ArrayRef,
129+
keys: &ArrayRef,
130+
values: &ArrayRef,
131131
can_evaluate_to_const: bool,
132-
data_type: DataType,
132+
data_type: &DataType,
133133
) -> Result<ColumnarValue> {
134134
if keys.len() != values.len() {
135135
return exec_err!("map requires key and value lists to have the same length");
@@ -159,8 +159,8 @@ fn make_map_batch_internal(
159159
let mut entry_offsets_buffer = VecDeque::new();
160160
entry_offsets_buffer.push_back(0);
161161

162-
entry_struct_buffer.push_back((Arc::clone(&key_field), Arc::clone(&keys)));
163-
entry_struct_buffer.push_back((Arc::clone(&value_field), Arc::clone(&values)));
162+
entry_struct_buffer.push_back((Arc::clone(&key_field), Arc::clone(keys)));
163+
entry_struct_buffer.push_back((Arc::clone(&value_field), Arc::clone(values)));
164164
entry_offsets_buffer.push_back(keys.len() as u32);
165165

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

383-
let keys = list_to_arrays::<O>(&keys);
384-
let values = list_to_arrays::<O>(&values);
383+
let keys = list_to_arrays::<O>(keys);
384+
let values = list_to_arrays::<O>(values);
385385

386386
build_map_array(
387-
keys,
388-
values,
389-
keys_data_type,
390-
values_data_type,
387+
&keys,
388+
&values,
389+
&keys_data_type,
390+
&values_data_type,
391391
original_len,
392392
nulls_bitmap,
393393
)
@@ -396,8 +396,8 @@ fn make_map_array_internal<O: OffsetSizeTrait>(
396396
/// Helper function specifically for FixedSizeList inputs
397397
/// Similar to make_map_array_internal but uses fixed_size_list_to_arrays instead of list_to_arrays
398398
fn make_map_array_from_fixed_size_list(
399-
keys: ArrayRef,
400-
values: ArrayRef,
399+
keys: &ArrayRef,
400+
values: &ArrayRef,
401401
) -> Result<ColumnarValue> {
402402
// Save original data types and array length
403403
let keys_data_type = keys.data_type().clone();
@@ -407,25 +407,25 @@ fn make_map_array_from_fixed_size_list(
407407
// Save the nulls bitmap from the original keys array
408408
let nulls_bitmap = keys.nulls().cloned();
409409

410-
let keys = fixed_size_list_to_arrays(&keys);
411-
let values = fixed_size_list_to_arrays(&values);
410+
let keys = fixed_size_list_to_arrays(keys);
411+
let values = fixed_size_list_to_arrays(values);
412412

413413
build_map_array(
414-
keys,
415-
values,
416-
keys_data_type,
417-
values_data_type,
414+
&keys,
415+
&values,
416+
&keys_data_type,
417+
&values_data_type,
418418
original_len,
419419
nulls_bitmap,
420420
)
421421
}
422422

423423
/// Common logic to build a MapArray from decomposed list arrays
424424
fn build_map_array(
425-
keys: Vec<ArrayRef>,
426-
values: Vec<ArrayRef>,
427-
keys_data_type: DataType,
428-
values_data_type: DataType,
425+
keys: &[ArrayRef],
426+
values: &[ArrayRef],
427+
keys_data_type: &DataType,
428+
values_data_type: &DataType,
429429
original_len: usize,
430430
nulls_bitmap: Option<arrow::buffer::NullBuffer>,
431431
) -> Result<ColumnarValue> {
@@ -470,8 +470,8 @@ fn build_map_array(
470470
let (flattened_keys, flattened_values) = if key_array_vec.is_empty() {
471471
// All maps are NULL - create empty arrays
472472
// We need to infer the data type from the original keys/values arrays
473-
let key_type = get_element_type(&keys_data_type)?;
474-
let value_type = get_element_type(&values_data_type)?;
473+
let key_type = get_element_type(keys_data_type)?;
474+
let value_type = get_element_type(values_data_type)?;
475475

476476
(
477477
arrow::array::new_empty_array(key_type),

datafusion/functions-nested/src/position.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,13 +181,13 @@ fn general_position_dispatch<O: OffsetSizeTrait>(args: &[ArrayRef]) -> Result<Ar
181181
);
182182
}
183183

184-
generic_position::<O>(list_array, element_array, arr_from)
184+
generic_position::<O>(list_array, element_array, &arr_from)
185185
}
186186

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

datafusion/functions-nested/src/remove.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,29 +289,29 @@ pub fn array_remove_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
289289
let [array, element] = take_function_args("array_remove", args)?;
290290

291291
let arr_n = vec![1; array.len()];
292-
array_remove_internal(array, element, arr_n)
292+
array_remove_internal(array, element, &arr_n)
293293
}
294294

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

299299
let arr_n = as_int64_array(max)?.values().to_vec();
300-
array_remove_internal(array, element, arr_n)
300+
array_remove_internal(array, element, &arr_n)
301301
}
302302

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

307307
let arr_n = vec![i64::MAX; array.len()];
308-
array_remove_internal(array, element, arr_n)
308+
array_remove_internal(array, element, &arr_n)
309309
}
310310

311311
fn array_remove_internal(
312312
array: &ArrayRef,
313313
element_array: &ArrayRef,
314-
arr_n: Vec<i64>,
314+
arr_n: &[i64],
315315
) -> Result<ArrayRef> {
316316
match array.data_type() {
317317
DataType::List(_) => {
@@ -348,7 +348,7 @@ fn array_remove_internal(
348348
fn general_remove<OffsetSize: OffsetSizeTrait>(
349349
list_array: &GenericListArray<OffsetSize>,
350350
element_array: &ArrayRef,
351-
arr_n: Vec<i64>,
351+
arr_n: &[i64],
352352
) -> Result<ArrayRef> {
353353
let data_type = list_array.value_type();
354354
let mut new_values = vec![];

0 commit comments

Comments
 (0)