Jefffrey commented on code in PR #18768:
URL: https://github.com/apache/datafusion/pull/18768#discussion_r2533923114
##########
datafusion/functions/src/core/greatest_least_utils.rs:
##########
@@ -102,8 +102,8 @@ pub(super) fn execute_conditional<Op:
GreatestLeastOperator>(
// Start with the result value
result = keep_array::<Op>(
- Arc::clone(first_array),
- result_scalar.to_array_of_size(first_array.len())?,
+ &Arc::clone(first_array),
Review Comment:
We should be carefully reviewing the changes made to see if they make sense;
in this case, passing the reference of an `Arc` we just cloned makes little
sense. Consider removing the clone.
##########
datafusion/functions/src/crypto/basic.rs:
##########
@@ -295,7 +295,7 @@ impl DigestAlgorithm {
pub fn digest_utf8_array_impl<'a, StringArrType>(
self,
- input_value: StringArrType,
+ input_value: &StringArrType,
Review Comment:
This change seems odd; `input_value` should already be a reference I believe?
##########
datafusion/functions/src/core/getfield.rs:
##########
@@ -197,7 +197,7 @@ impl ScalarUDFImpl for GetFieldFunc {
};
fn process_map_array(
- array: Arc<dyn Array>,
+ array: &Arc<dyn Array>,
Review Comment:
Perhaps its better to pass `&dyn Array`?
##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -145,12 +145,12 @@ impl ScalarUDFImpl for ToCharFunc {
match format {
ColumnarValue::Scalar(ScalarValue::Utf8(None))
| ColumnarValue::Scalar(ScalarValue::Null) => {
- to_char_scalar(date_time.clone(), None)
+ to_char_scalar(&date_time.clone(), None)
Review Comment:
Another example of needless cloning; there's other occurrences in this PR
##########
datafusion/functions/src/regex/regexpcount.rs:
##########
@@ -260,6 +260,7 @@ pub fn regexp_count(
}
}
+#[expect(clippy::needless_pass_by_value)]
pub fn regexp_count_inner<'a, S>(
Review Comment:
Honestly I'm not sure this is meant to be a public function anyway
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
