Skip to content

Commit 1e9c1f4

Browse files
authored
minor: enforce lint rule clippy::needless_pass_by_value to datafusion-functions (#18768)
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123. --> - Closes #18758. ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> - enforce lint rule `clippy::needless_pass_by_value` to `datafusion-functions`. ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. --> --------- Signed-off-by: StandingMan <jmtangcs@gmail.com>
1 parent 5e36b05 commit 1e9c1f4

File tree

20 files changed

+114
-104
lines changed

20 files changed

+114
-104
lines changed

datafusion/functions/src/core/getfield.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -197,10 +197,10 @@ impl ScalarUDFImpl for GetFieldFunc {
197197
};
198198

199199
fn process_map_array(
200-
array: Arc<dyn Array>,
200+
array: &dyn Array,
201201
key_array: Arc<dyn Array>,
202202
) -> Result<ColumnarValue> {
203-
let map_array = as_map_array(array.as_ref())?;
203+
let map_array = as_map_array(array)?;
204204
let keys = if key_array.data_type().is_nested() {
205205
let comparator = make_comparator(
206206
map_array.keys().as_ref(),
@@ -246,14 +246,14 @@ impl ScalarUDFImpl for GetFieldFunc {
246246
}
247247

248248
fn process_map_with_nested_key(
249-
array: Arc<dyn Array>,
250-
key_array: Arc<dyn Array>,
249+
array: &dyn Array,
250+
key_array: &dyn Array,
251251
) -> Result<ColumnarValue> {
252-
let map_array = as_map_array(array.as_ref())?;
252+
let map_array = as_map_array(array)?;
253253

254254
let comparator = make_comparator(
255255
map_array.keys().as_ref(),
256-
key_array.as_ref(),
256+
key_array,
257257
SortOptions::default(),
258258
)?;
259259

@@ -288,17 +288,17 @@ impl ScalarUDFImpl for GetFieldFunc {
288288
match (array.data_type(), name) {
289289
(DataType::Map(_, _), ScalarValue::List(arr)) => {
290290
let key_array: Arc<dyn Array> = arr;
291-
process_map_array(array, key_array)
291+
process_map_array(&array, key_array)
292292
}
293293
(DataType::Map(_, _), ScalarValue::Struct(arr)) => {
294-
process_map_array(array, arr as Arc<dyn Array>)
294+
process_map_array(&array, arr as Arc<dyn Array>)
295295
}
296296
(DataType::Map(_, _), other) => {
297297
let data_type = other.data_type();
298298
if data_type.is_nested() {
299-
process_map_with_nested_key(array, other.to_array()?)
299+
process_map_with_nested_key(&array, &other.to_array()?)
300300
} else {
301-
process_map_array(array, other.to_array()?)
301+
process_map_array(&array, other.to_array()?)
302302
}
303303
}
304304
(DataType::Struct(_), ScalarValue::Utf8(Some(k))) => {

datafusion/functions/src/core/greatest_least_utils.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ pub(super) trait GreatestLeastOperator {
3838
}
3939

4040
fn keep_array<Op: GreatestLeastOperator>(
41-
lhs: ArrayRef,
42-
rhs: ArrayRef,
41+
lhs: &dyn Array,
42+
rhs: &dyn Array,
4343
) -> Result<ArrayRef> {
4444
// True for values that we should keep from the left array
45-
let keep_lhs = Op::get_indexes_to_keep(lhs.as_ref(), rhs.as_ref())?;
45+
let keep_lhs = Op::get_indexes_to_keep(lhs, rhs)?;
4646

4747
let result = zip(&keep_lhs, &lhs, &rhs)?;
4848

@@ -102,8 +102,8 @@ pub(super) fn execute_conditional<Op: GreatestLeastOperator>(
102102

103103
// Start with the result value
104104
result = keep_array::<Op>(
105-
Arc::clone(first_array),
106-
result_scalar.to_array_of_size(first_array.len())?,
105+
first_array,
106+
&result_scalar.to_array_of_size(first_array.len())?,
107107
)?;
108108
} else {
109109
// If we only have arrays, start with the first array
@@ -112,7 +112,7 @@ pub(super) fn execute_conditional<Op: GreatestLeastOperator>(
112112
}
113113

114114
for array in arrays_iter {
115-
result = keep_array::<Op>(Arc::clone(array), result)?;
115+
result = keep_array::<Op>(array, &result)?;
116116
}
117117

118118
Ok(ColumnarValue::Array(result))

datafusion/functions/src/core/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,13 @@ pub mod expr_fn {
110110
));
111111

112112
#[doc = "Returns the value of the field with the given name from the struct"]
113+
#[expect(clippy::needless_pass_by_value)]
113114
pub fn get_field(arg1: Expr, arg2: impl Literal) -> Expr {
114115
super::get_field().call(vec![arg1, arg2.lit()])
115116
}
116117

117118
#[doc = "Returns the value of the field with the given name from the union when it's selected, or NULL otherwise"]
119+
#[expect(clippy::needless_pass_by_value)]
118120
pub fn union_extract(arg1: Expr, arg2: impl Literal) -> Expr {
119121
super::union_extract().call(vec![arg1, arg2.lit()])
120122
}

datafusion/functions/src/crypto/basic.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -259,11 +259,11 @@ impl DigestAlgorithm {
259259
let array = match value.data_type() {
260260
DataType::Binary | DataType::LargeBinary => {
261261
let v = value.as_binary::<T>();
262-
self.digest_binary_array_impl::<&GenericBinaryArray<T>>(v)
262+
self.digest_binary_array_impl::<&GenericBinaryArray<T>>(&v)
263263
}
264264
DataType::BinaryView => {
265265
let v = value.as_binary_view();
266-
self.digest_binary_array_impl::<&BinaryViewArray>(v)
266+
self.digest_binary_array_impl::<&BinaryViewArray>(&v)
267267
}
268268
other => {
269269
return exec_err!("unsupported type for digest_utf_array: {other:?}")
@@ -280,11 +280,11 @@ impl DigestAlgorithm {
280280
let array = match value.data_type() {
281281
DataType::Utf8 | DataType::LargeUtf8 => {
282282
let v = value.as_string::<T>();
283-
self.digest_utf8_array_impl::<&GenericStringArray<T>>(v)
283+
self.digest_utf8_array_impl::<&GenericStringArray<T>>(&v)
284284
}
285285
DataType::Utf8View => {
286286
let v = value.as_string_view();
287-
self.digest_utf8_array_impl::<&StringViewArray>(v)
287+
self.digest_utf8_array_impl::<&StringViewArray>(&v)
288288
}
289289
other => {
290290
return exec_err!("unsupported type for digest_utf_array: {other:?}")
@@ -295,7 +295,7 @@ impl DigestAlgorithm {
295295

296296
pub fn digest_utf8_array_impl<'a, StringArrType>(
297297
self,
298-
input_value: StringArrType,
298+
input_value: &StringArrType,
299299
) -> ArrayRef
300300
where
301301
StringArrType: StringArrayType<'a>,
@@ -326,7 +326,7 @@ impl DigestAlgorithm {
326326

327327
pub fn digest_binary_array_impl<'a, BinaryArrType>(
328328
self,
329-
input_value: BinaryArrType,
329+
input_value: &BinaryArrType,
330330
) -> ArrayRef
331331
where
332332
BinaryArrType: BinaryArrayType<'a>,

datafusion/functions/src/datetime/common.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,19 +190,19 @@ where
190190
ColumnarValue::Array(a) => match a.data_type() {
191191
DataType::Utf8View => Ok(ColumnarValue::Array(Arc::new(
192192
unary_string_to_primitive_function::<&StringViewArray, O, _>(
193-
a.as_ref().as_string_view(),
193+
&a.as_string_view(),
194194
op,
195195
)?,
196196
))),
197197
DataType::LargeUtf8 => Ok(ColumnarValue::Array(Arc::new(
198198
unary_string_to_primitive_function::<&GenericStringArray<i64>, O, _>(
199-
a.as_ref().as_string::<i64>(),
199+
&a.as_string::<i64>(),
200200
op,
201201
)?,
202202
))),
203203
DataType::Utf8 => Ok(ColumnarValue::Array(Arc::new(
204204
unary_string_to_primitive_function::<&GenericStringArray<i32>, O, _>(
205-
a.as_ref().as_string::<i32>(),
205+
&a.as_string::<i32>(),
206206
op,
207207
)?,
208208
))),
@@ -431,7 +431,7 @@ where
431431
/// * the number of arguments is not 1 or
432432
/// * the function `op` errors
433433
fn unary_string_to_primitive_function<'a, StringArrType, O, F>(
434-
array: StringArrType,
434+
array: &StringArrType,
435435
op: F,
436436
) -> Result<PrimitiveArray<O>>
437437
where

datafusion/functions/src/datetime/to_char.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -144,13 +144,11 @@ impl ScalarUDFImpl for ToCharFunc {
144144

145145
match format {
146146
ColumnarValue::Scalar(ScalarValue::Utf8(None))
147-
| ColumnarValue::Scalar(ScalarValue::Null) => {
148-
to_char_scalar(date_time.clone(), None)
149-
}
147+
| ColumnarValue::Scalar(ScalarValue::Null) => to_char_scalar(date_time, None),
150148
// constant format
151149
ColumnarValue::Scalar(ScalarValue::Utf8(Some(format))) => {
152150
// invoke to_char_scalar with the known string, without converting to array
153-
to_char_scalar(date_time.clone(), Some(format))
151+
to_char_scalar(date_time, Some(format))
154152
}
155153
ColumnarValue::Array(_) => to_char_array(&args),
156154
_ => {
@@ -206,7 +204,7 @@ fn build_format_options<'a>(
206204

207205
/// Special version when arg\[1] is a scalar
208206
fn to_char_scalar(
209-
expression: ColumnarValue,
207+
expression: &ColumnarValue,
210208
format: Option<&str>,
211209
) -> Result<ColumnarValue> {
212210
// it's possible that the expression is a scalar however because
@@ -253,7 +251,7 @@ fn to_char_scalar(
253251
// if the data type was a Date32, formatting could have failed because the format string
254252
// contained datetime specifiers, so we'll retry by casting the date array as a timestamp array
255253
if data_type == &Date32 {
256-
return to_char_scalar(expression.clone().cast_to(&Date64, None)?, format);
254+
return to_char_scalar(&expression.cast_to(&Date64, None)?, format);
257255
}
258256

259257
exec_err!("{}", formatted.unwrap_err())
@@ -292,7 +290,7 @@ fn to_char_array(args: &[ColumnarValue]) -> Result<ColumnarValue> {
292290
if data_type == &Date32 {
293291
let failed_date_value = arrays[0].slice(idx, 1);
294292

295-
match retry_date_as_timestamp(failed_date_value, &format_options) {
293+
match retry_date_as_timestamp(&failed_date_value, &format_options) {
296294
Ok(value) => {
297295
results.push(Some(value));
298296
continue;
@@ -322,7 +320,7 @@ fn to_char_array(args: &[ColumnarValue]) -> Result<ColumnarValue> {
322320
}
323321

324322
fn retry_date_as_timestamp(
325-
array_ref: ArrayRef,
323+
array_ref: &ArrayRef,
326324
format_options: &FormatOptions,
327325
) -> Result<String> {
328326
let target_data_type = Date64;

datafusion/functions/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
//! Function packages for [DataFusion].
2831
//!

datafusion/functions/src/regex/regexpcount.rs

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -201,57 +201,57 @@ pub fn regexp_count(
201201

202202
match (values.data_type(), regex_array.data_type(), flags_array) {
203203
(Utf8, Utf8, None) => regexp_count_inner(
204-
values.as_string::<i32>(),
205-
regex_array.as_string::<i32>(),
204+
&values.as_string::<i32>(),
205+
&regex_array.as_string::<i32>(),
206206
is_regex_scalar,
207207
start_array.map(|start| start.as_primitive::<Int64Type>()),
208208
is_start_scalar,
209209
None,
210210
is_flags_scalar,
211211
),
212212
(Utf8, Utf8, Some(flags_array)) if *flags_array.data_type() == Utf8 => regexp_count_inner(
213-
values.as_string::<i32>(),
214-
regex_array.as_string::<i32>(),
213+
&values.as_string::<i32>(),
214+
&regex_array.as_string::<i32>(),
215215
is_regex_scalar,
216216
start_array.map(|start| start.as_primitive::<Int64Type>()),
217217
is_start_scalar,
218-
Some(flags_array.as_string::<i32>()),
218+
Some(&flags_array.as_string::<i32>()),
219219
is_flags_scalar,
220220
),
221221
(LargeUtf8, LargeUtf8, None) => regexp_count_inner(
222-
values.as_string::<i64>(),
223-
regex_array.as_string::<i64>(),
222+
&values.as_string::<i64>(),
223+
&regex_array.as_string::<i64>(),
224224
is_regex_scalar,
225225
start_array.map(|start| start.as_primitive::<Int64Type>()),
226226
is_start_scalar,
227227
None,
228228
is_flags_scalar,
229229
),
230230
(LargeUtf8, LargeUtf8, Some(flags_array)) if *flags_array.data_type() == LargeUtf8 => regexp_count_inner(
231-
values.as_string::<i64>(),
232-
regex_array.as_string::<i64>(),
231+
&values.as_string::<i64>(),
232+
&regex_array.as_string::<i64>(),
233233
is_regex_scalar,
234234
start_array.map(|start| start.as_primitive::<Int64Type>()),
235235
is_start_scalar,
236-
Some(flags_array.as_string::<i64>()),
236+
Some(&flags_array.as_string::<i64>()),
237237
is_flags_scalar,
238238
),
239239
(Utf8View, Utf8View, None) => regexp_count_inner(
240-
values.as_string_view(),
241-
regex_array.as_string_view(),
240+
&values.as_string_view(),
241+
&regex_array.as_string_view(),
242242
is_regex_scalar,
243243
start_array.map(|start| start.as_primitive::<Int64Type>()),
244244
is_start_scalar,
245245
None,
246246
is_flags_scalar,
247247
),
248248
(Utf8View, Utf8View, Some(flags_array)) if *flags_array.data_type() == Utf8View => regexp_count_inner(
249-
values.as_string_view(),
250-
regex_array.as_string_view(),
249+
&values.as_string_view(),
250+
&regex_array.as_string_view(),
251251
is_regex_scalar,
252252
start_array.map(|start| start.as_primitive::<Int64Type>()),
253253
is_start_scalar,
254-
Some(flags_array.as_string_view()),
254+
Some(&flags_array.as_string_view()),
255255
is_flags_scalar,
256256
),
257257
_ => Err(ArrowError::ComputeError(
@@ -260,13 +260,13 @@ pub fn regexp_count(
260260
}
261261
}
262262

263-
pub fn regexp_count_inner<'a, S>(
264-
values: S,
265-
regex_array: S,
263+
fn regexp_count_inner<'a, S>(
264+
values: &S,
265+
regex_array: &S,
266266
is_regex_scalar: bool,
267267
start_array: Option<&Int64Array>,
268268
is_start_scalar: bool,
269-
flags_array: Option<S>,
269+
flags_array: Option<&S>,
270270
is_flags_scalar: bool,
271271
) -> Result<ArrayRef, ArrowError>
272272
where

datafusion/functions/src/regex/regexpinstr.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ pub fn regexp_instr(
287287
}
288288

289289
#[allow(clippy::too_many_arguments)]
290+
#[expect(clippy::needless_pass_by_value)]
290291
pub fn regexp_instr_inner<'a, S>(
291292
values: S,
292293
regex_array: S,

datafusion/functions/src/regex/regexpreplace.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ where
382382
}
383383
}
384384

385+
#[expect(clippy::needless_pass_by_value)]
385386
fn _regexp_replace_early_abort<T: ArrayAccessor>(
386387
input_array: T,
387388
sz: usize,

0 commit comments

Comments
 (0)