This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new f204869ff5 Enable `clone_on_ref_ptr` clippy lint on functions* (#11468)
f204869ff5 is described below
commit f204869ff55bb3e39cf23fc0a34ebd5021e6773f
Author: 张林伟 <[email protected]>
AuthorDate: Tue Jul 16 02:54:10 2024 +0800
Enable `clone_on_ref_ptr` clippy lint on functions* (#11468)
* Enable clone_on_ref_ptr clippy lint on functions
* Remove unnecessary Arc::clone
---
datafusion/functions-aggregate/src/correlation.rs | 21 ++++++++++++++------
datafusion/functions-aggregate/src/first_last.rs | 16 +++++++--------
datafusion/functions-aggregate/src/lib.rs | 2 ++
datafusion/functions-array/src/array_has.rs | 4 ++--
datafusion/functions-array/src/concat.rs | 2 +-
datafusion/functions-array/src/flatten.rs | 4 ++--
datafusion/functions-array/src/lib.rs | 2 ++
datafusion/functions-array/src/resize.rs | 8 ++++----
datafusion/functions-array/src/reverse.rs | 4 ++--
datafusion/functions-array/src/set_ops.rs | 12 ++++++------
datafusion/functions-array/src/sort.rs | 2 +-
datafusion/functions-array/src/string.rs | 2 +-
datafusion/functions-array/src/utils.rs | 14 +++++++------
datafusion/functions/benches/concat.rs | 3 ++-
datafusion/functions/benches/regx.rs | 24 +++++++++++++++--------
datafusion/functions/src/core/getfield.rs | 5 +++--
datafusion/functions/src/core/map.rs | 6 +++---
datafusion/functions/src/core/nvl.rs | 7 ++++---
datafusion/functions/src/core/nvl2.rs | 3 ++-
datafusion/functions/src/core/struct.rs | 17 ++++------------
datafusion/functions/src/datetime/date_part.rs | 2 +-
datafusion/functions/src/datetime/to_timestamp.rs | 21 ++++++++++----------
datafusion/functions/src/lib.rs | 2 ++
datafusion/functions/src/math/abs.rs | 2 +-
datafusion/functions/src/math/log.rs | 2 +-
datafusion/functions/src/math/round.rs | 2 +-
datafusion/functions/src/math/trunc.rs | 2 +-
27 files changed, 106 insertions(+), 85 deletions(-)
diff --git a/datafusion/functions-aggregate/src/correlation.rs
b/datafusion/functions-aggregate/src/correlation.rs
index 10d5563086..c2d7a89081 100644
--- a/datafusion/functions-aggregate/src/correlation.rs
+++ b/datafusion/functions-aggregate/src/correlation.rs
@@ -19,6 +19,7 @@
use std::any::Any;
use std::fmt::Debug;
+use std::sync::Arc;
use arrow::compute::{and, filter, is_not_null};
use arrow::{
@@ -192,13 +193,21 @@ impl Accumulator for CorrelationAccumulator {
fn merge_batch(&mut self, states: &[ArrayRef]) -> Result<()> {
let states_c = [
- states[0].clone(),
- states[1].clone(),
- states[3].clone(),
- states[5].clone(),
+ Arc::clone(&states[0]),
+ Arc::clone(&states[1]),
+ Arc::clone(&states[3]),
+ Arc::clone(&states[5]),
+ ];
+ let states_s1 = [
+ Arc::clone(&states[0]),
+ Arc::clone(&states[1]),
+ Arc::clone(&states[2]),
+ ];
+ let states_s2 = [
+ Arc::clone(&states[0]),
+ Arc::clone(&states[3]),
+ Arc::clone(&states[4]),
];
- let states_s1 = [states[0].clone(), states[1].clone(),
states[2].clone()];
- let states_s2 = [states[0].clone(), states[3].clone(),
states[4].clone()];
self.covar.merge_batch(&states_c)?;
self.stddev1.merge_batch(&states_s1)?;
diff --git a/datafusion/functions-aggregate/src/first_last.rs
b/datafusion/functions-aggregate/src/first_last.rs
index dd38e34872..0e619bacef 100644
--- a/datafusion/functions-aggregate/src/first_last.rs
+++ b/datafusion/functions-aggregate/src/first_last.rs
@@ -247,7 +247,7 @@ impl FirstValueAccumulator {
.iter()
.zip(self.ordering_req.iter())
.map(|(values, req)| SortColumn {
- values: values.clone(),
+ values: Arc::clone(values),
options: Some(req.options),
})
.collect::<Vec<_>>();
@@ -547,7 +547,7 @@ impl LastValueAccumulator {
// Take the reverse ordering requirement. This enables us to
// use "fetch = 1" to get the last value.
SortColumn {
- values: values.clone(),
+ values: Arc::clone(values),
options: Some(!req.options),
}
})
@@ -676,7 +676,7 @@ fn convert_to_sort_cols(
arrs.iter()
.zip(sort_exprs.iter())
.map(|(item, sort_expr)| SortColumn {
- values: item.clone(),
+ values: Arc::clone(item),
options: Some(sort_expr.options),
})
.collect::<Vec<_>>()
@@ -707,7 +707,7 @@ mod tests {
for arr in arrs {
// Once first_value is set, accumulator should remember it.
// It shouldn't update first_value for each new batch
- first_accumulator.update_batch(&[arr.clone()])?;
+ first_accumulator.update_batch(&[Arc::clone(&arr)])?;
// last_value should be updated for each new batch.
last_accumulator.update_batch(&[arr])?;
}
@@ -733,12 +733,12 @@ mod tests {
let mut first_accumulator =
FirstValueAccumulator::try_new(&DataType::Int64, &[], vec![],
false)?;
- first_accumulator.update_batch(&[arrs[0].clone()])?;
+ first_accumulator.update_batch(&[Arc::clone(&arrs[0])])?;
let state1 = first_accumulator.state()?;
let mut first_accumulator =
FirstValueAccumulator::try_new(&DataType::Int64, &[], vec![],
false)?;
- first_accumulator.update_batch(&[arrs[1].clone()])?;
+ first_accumulator.update_batch(&[Arc::clone(&arrs[1])])?;
let state2 = first_accumulator.state()?;
assert_eq!(state1.len(), state2.len());
@@ -763,12 +763,12 @@ mod tests {
let mut last_accumulator =
LastValueAccumulator::try_new(&DataType::Int64, &[], vec![],
false)?;
- last_accumulator.update_batch(&[arrs[0].clone()])?;
+ last_accumulator.update_batch(&[Arc::clone(&arrs[0])])?;
let state1 = last_accumulator.state()?;
let mut last_accumulator =
LastValueAccumulator::try_new(&DataType::Int64, &[], vec![],
false)?;
- last_accumulator.update_batch(&[arrs[1].clone()])?;
+ last_accumulator.update_batch(&[Arc::clone(&arrs[1])])?;
let state2 = last_accumulator.state()?;
assert_eq!(state1.len(), state2.len());
diff --git a/datafusion/functions-aggregate/src/lib.rs
b/datafusion/functions-aggregate/src/lib.rs
index 6ae2dfb369..a3808a08b0 100644
--- a/datafusion/functions-aggregate/src/lib.rs
+++ b/datafusion/functions-aggregate/src/lib.rs
@@ -14,6 +14,8 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
+// Make cheap clones clear: https://github.com/apache/datafusion/issues/11143
+#![deny(clippy::clone_on_ref_ptr)]
//! Aggregate Function packages for [DataFusion].
//!
diff --git a/datafusion/functions-array/src/array_has.rs
b/datafusion/functions-array/src/array_has.rs
index 136c6e7691..bdda5a5659 100644
--- a/datafusion/functions-array/src/array_has.rs
+++ b/datafusion/functions-array/src/array_has.rs
@@ -279,7 +279,7 @@ fn general_array_has_dispatch<O: OffsetSizeTrait>(
let converter =
RowConverter::new(vec![SortField::new(array.value_type())])?;
- let element = sub_array.clone();
+ let element = Arc::clone(sub_array);
let sub_array = if comparison_type != ComparisonType::Single {
as_generic_list_array::<O>(sub_array)?
} else {
@@ -292,7 +292,7 @@ fn general_array_has_dispatch<O: OffsetSizeTrait>(
let sub_arr_values = if comparison_type !=
ComparisonType::Single {
converter.convert_columns(&[sub_arr])?
} else {
- converter.convert_columns(&[element.clone()])?
+ converter.convert_columns(&[Arc::clone(&element)])?
};
let mut res = match comparison_type {
diff --git a/datafusion/functions-array/src/concat.rs
b/datafusion/functions-array/src/concat.rs
index 330c50f5b0..c52118d0a5 100644
--- a/datafusion/functions-array/src/concat.rs
+++ b/datafusion/functions-array/src/concat.rs
@@ -249,7 +249,7 @@ pub(crate) fn array_concat_inner(args: &[ArrayRef]) ->
Result<ArrayRef> {
return not_impl_err!("Array is not type '{base_type:?}'.");
}
if !base_type.eq(&DataType::Null) {
- new_args.push(arg.clone());
+ new_args.push(Arc::clone(arg));
}
}
diff --git a/datafusion/functions-array/src/flatten.rs
b/datafusion/functions-array/src/flatten.rs
index a495c3ade9..2b383af3d4 100644
--- a/datafusion/functions-array/src/flatten.rs
+++ b/datafusion/functions-array/src/flatten.rs
@@ -77,7 +77,7 @@ impl ScalarUDFImpl for Flatten {
get_base_type(field.data_type())
}
Null | List(_) | LargeList(_) => Ok(data_type.to_owned()),
- FixedSizeList(field, _) => Ok(List(field.clone())),
+ FixedSizeList(field, _) => Ok(List(Arc::clone(field))),
_ => exec_err!(
"Not reachable, data_type should be List, LargeList or
FixedSizeList"
),
@@ -115,7 +115,7 @@ pub fn flatten_inner(args: &[ArrayRef]) -> Result<ArrayRef>
{
let flattened_array = flatten_internal::<i64>(list_arr.clone(),
None)?;
Ok(Arc::new(flattened_array) as ArrayRef)
}
- Null => Ok(args[0].clone()),
+ Null => Ok(Arc::clone(&args[0])),
_ => {
exec_err!("flatten does not support type '{array_type:?}'")
}
diff --git a/datafusion/functions-array/src/lib.rs
b/datafusion/functions-array/src/lib.rs
index 814127be80..9717d29883 100644
--- a/datafusion/functions-array/src/lib.rs
+++ b/datafusion/functions-array/src/lib.rs
@@ -14,6 +14,8 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
+// Make cheap clones clear: https://github.com/apache/datafusion/issues/11143
+#![deny(clippy::clone_on_ref_ptr)]
//! Array Functions for [DataFusion].
//!
diff --git a/datafusion/functions-array/src/resize.rs
b/datafusion/functions-array/src/resize.rs
index 078ec7766a..83c545a26e 100644
--- a/datafusion/functions-array/src/resize.rs
+++ b/datafusion/functions-array/src/resize.rs
@@ -67,8 +67,8 @@ impl ScalarUDFImpl for ArrayResize {
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
match &arg_types[0] {
- List(field) | FixedSizeList(field, _) => Ok(List(field.clone())),
- LargeList(field) => Ok(LargeList(field.clone())),
+ List(field) | FixedSizeList(field, _) =>
Ok(List(Arc::clone(field))),
+ LargeList(field) => Ok(LargeList(Arc::clone(field))),
_ => exec_err!(
"Not reachable, data_type should be List, LargeList or
FixedSizeList"
),
@@ -92,7 +92,7 @@ pub(crate) fn array_resize_inner(arg: &[ArrayRef]) ->
Result<ArrayRef> {
let new_len = as_int64_array(&arg[1])?;
let new_element = if arg.len() == 3 {
- Some(arg[2].clone())
+ Some(Arc::clone(&arg[2]))
} else {
None
};
@@ -168,7 +168,7 @@ fn general_list_resize<O: OffsetSizeTrait + TryInto<i64>>(
let data = mutable.freeze();
Ok(Arc::new(GenericListArray::<O>::try_new(
- field.clone(),
+ Arc::clone(field),
OffsetBuffer::<O>::new(offsets.into()),
arrow_array::make_array(data),
None,
diff --git a/datafusion/functions-array/src/reverse.rs
b/datafusion/functions-array/src/reverse.rs
index b462be4020..581caf5daf 100644
--- a/datafusion/functions-array/src/reverse.rs
+++ b/datafusion/functions-array/src/reverse.rs
@@ -93,7 +93,7 @@ pub fn array_reverse_inner(arg: &[ArrayRef]) ->
Result<ArrayRef> {
let array = as_large_list_array(&arg[0])?;
general_array_reverse::<i64>(array, field)
}
- Null => Ok(arg[0].clone()),
+ Null => Ok(Arc::clone(&arg[0])),
array_type => exec_err!("array_reverse does not support type
'{array_type:?}'."),
}
}
@@ -137,7 +137,7 @@ fn general_array_reverse<O: OffsetSizeTrait + TryFrom<i64>>(
let data = mutable.freeze();
Ok(Arc::new(GenericListArray::<O>::try_new(
- field.clone(),
+ Arc::clone(field),
OffsetBuffer::<O>::new(offsets.into()),
arrow_array::make_array(data),
Some(nulls.into()),
diff --git a/datafusion/functions-array/src/set_ops.rs
b/datafusion/functions-array/src/set_ops.rs
index a843a175f3..1de9c264dd 100644
--- a/datafusion/functions-array/src/set_ops.rs
+++ b/datafusion/functions-array/src/set_ops.rs
@@ -213,7 +213,7 @@ fn array_distinct_inner(args: &[ArrayRef]) ->
Result<ArrayRef> {
// handle null
if args[0].data_type() == &Null {
- return Ok(args[0].clone());
+ return Ok(Arc::clone(&args[0]));
}
// handle for list & largelist
@@ -314,7 +314,7 @@ fn generic_set_lists<OffsetSize: OffsetSizeTrait>(
offsets.push(last_offset + OffsetSize::usize_as(rows.len()));
let arrays = converter.convert_rows(rows)?;
let array = match arrays.first() {
- Some(array) => array.clone(),
+ Some(array) => Arc::clone(array),
None => {
return internal_err!("{set_op}: failed to get array from
rows");
}
@@ -370,12 +370,12 @@ fn general_set_op(
(List(field), List(_)) => {
let array1 = as_list_array(&array1)?;
let array2 = as_list_array(&array2)?;
- generic_set_lists::<i32>(array1, array2, field.clone(), set_op)
+ generic_set_lists::<i32>(array1, array2, Arc::clone(field), set_op)
}
(LargeList(field), LargeList(_)) => {
let array1 = as_large_list_array(&array1)?;
let array2 = as_large_list_array(&array2)?;
- generic_set_lists::<i64>(array1, array2, field.clone(), set_op)
+ generic_set_lists::<i64>(array1, array2, Arc::clone(field), set_op)
}
(data_type1, data_type2) => {
internal_err!(
@@ -426,7 +426,7 @@ fn general_array_distinct<OffsetSize: OffsetSizeTrait>(
offsets.push(last_offset + OffsetSize::usize_as(rows.len()));
let arrays = converter.convert_rows(rows)?;
let array = match arrays.first() {
- Some(array) => array.clone(),
+ Some(array) => Arc::clone(array),
None => {
return internal_err!("array_distinct: failed to get array from
rows")
}
@@ -437,7 +437,7 @@ fn general_array_distinct<OffsetSize: OffsetSizeTrait>(
let new_arrays_ref = new_arrays.iter().map(|v|
v.as_ref()).collect::<Vec<_>>();
let values = compute::concat(&new_arrays_ref)?;
Ok(Arc::new(GenericListArray::<OffsetSize>::try_new(
- field.clone(),
+ Arc::clone(field),
offsets,
values,
None,
diff --git a/datafusion/functions-array/src/sort.rs
b/datafusion/functions-array/src/sort.rs
index c82dbd37be..9c1ae50763 100644
--- a/datafusion/functions-array/src/sort.rs
+++ b/datafusion/functions-array/src/sort.rs
@@ -121,7 +121,7 @@ pub fn array_sort_inner(args: &[ArrayRef]) ->
Result<ArrayRef> {
let list_array = as_list_array(&args[0])?;
let row_count = list_array.len();
if row_count == 0 {
- return Ok(args[0].clone());
+ return Ok(Arc::clone(&args[0]));
}
let mut array_lengths = vec![];
diff --git a/datafusion/functions-array/src/string.rs
b/datafusion/functions-array/src/string.rs
index d02c863db8..2dc0a55e69 100644
--- a/datafusion/functions-array/src/string.rs
+++ b/datafusion/functions-array/src/string.rs
@@ -381,7 +381,7 @@ pub(super) fn array_to_string_inner(args: &[ArrayRef]) ->
Result<ArrayRef> {
let delimiter = delimiters[0].unwrap();
let s = compute_array_to_string(
&mut arg,
- arr.clone(),
+ Arc::clone(arr),
delimiter.to_string(),
null_string,
with_null_string,
diff --git a/datafusion/functions-array/src/utils.rs
b/datafusion/functions-array/src/utils.rs
index 3ecccf3c87..f396c3b225 100644
--- a/datafusion/functions-array/src/utils.rs
+++ b/datafusion/functions-array/src/utils.rs
@@ -105,7 +105,7 @@ pub(crate) fn align_array_dimensions<O: OffsetSizeTrait>(
.zip(args_ndim.iter())
.map(|(array, ndim)| {
if ndim < max_ndim {
- let mut aligned_array = array.clone();
+ let mut aligned_array = Arc::clone(&array);
for _ in 0..(max_ndim - ndim) {
let data_type = aligned_array.data_type().to_owned();
let array_lengths = vec![1; aligned_array.len()];
@@ -120,7 +120,7 @@ pub(crate) fn align_array_dimensions<O: OffsetSizeTrait>(
}
Ok(aligned_array)
} else {
- Ok(array.clone())
+ Ok(Arc::clone(&array))
}
})
.collect();
@@ -277,10 +277,12 @@ mod tests {
Some(vec![Some(6), Some(7), Some(8)]),
]));
- let array2d_1 =
- Arc::new(array_into_list_array_nullable(array1d_1.clone())) as
ArrayRef;
- let array2d_2 =
- Arc::new(array_into_list_array_nullable(array1d_2.clone())) as
ArrayRef;
+ let array2d_1 = Arc::new(array_into_list_array_nullable(
+ Arc::clone(&array1d_1) as ArrayRef
+ )) as ArrayRef;
+ let array2d_2 = Arc::new(array_into_list_array_nullable(
+ Arc::clone(&array1d_2) as ArrayRef
+ )) as ArrayRef;
let res = align_array_dimensions::<i32>(vec![
array1d_1.to_owned(),
diff --git a/datafusion/functions/benches/concat.rs
b/datafusion/functions/benches/concat.rs
index e7b00a6d54..91c46ac775 100644
--- a/datafusion/functions/benches/concat.rs
+++ b/datafusion/functions/benches/concat.rs
@@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.
+use arrow::array::ArrayRef;
use arrow::util::bench_util::create_string_array_with_len;
use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
use datafusion_common::ScalarValue;
@@ -26,7 +27,7 @@ fn create_args(size: usize, str_len: usize) ->
Vec<ColumnarValue> {
let array = Arc::new(create_string_array_with_len::<i32>(size, 0.2,
str_len));
let scalar = ScalarValue::Utf8(Some(", ".to_string()));
vec![
- ColumnarValue::Array(array.clone()),
+ ColumnarValue::Array(Arc::clone(&array) as ArrayRef),
ColumnarValue::Scalar(scalar),
ColumnarValue::Array(array),
]
diff --git a/datafusion/functions/benches/regx.rs
b/datafusion/functions/benches/regx.rs
index da4882381e..23d57f38ef 100644
--- a/datafusion/functions/benches/regx.rs
+++ b/datafusion/functions/benches/regx.rs
@@ -83,8 +83,12 @@ fn criterion_benchmark(c: &mut Criterion) {
b.iter(|| {
black_box(
- regexp_like::<i32>(&[data.clone(), regex.clone(),
flags.clone()])
- .expect("regexp_like should work on valid values"),
+ regexp_like::<i32>(&[
+ Arc::clone(&data),
+ Arc::clone(®ex),
+ Arc::clone(&flags),
+ ])
+ .expect("regexp_like should work on valid values"),
)
})
});
@@ -97,8 +101,12 @@ fn criterion_benchmark(c: &mut Criterion) {
b.iter(|| {
black_box(
- regexp_match::<i32>(&[data.clone(), regex.clone(),
flags.clone()])
- .expect("regexp_match should work on valid values"),
+ regexp_match::<i32>(&[
+ Arc::clone(&data),
+ Arc::clone(®ex),
+ Arc::clone(&flags),
+ ])
+ .expect("regexp_match should work on valid values"),
)
})
});
@@ -115,10 +123,10 @@ fn criterion_benchmark(c: &mut Criterion) {
b.iter(|| {
black_box(
regexp_replace::<i32>(&[
- data.clone(),
- regex.clone(),
- replacement.clone(),
- flags.clone(),
+ Arc::clone(&data),
+ Arc::clone(®ex),
+ Arc::clone(&replacement),
+ Arc::clone(&flags),
])
.expect("regexp_replace should work on valid values"),
)
diff --git a/datafusion/functions/src/core/getfield.rs
b/datafusion/functions/src/core/getfield.rs
index b76da15c52..2c2e36b91b 100644
--- a/datafusion/functions/src/core/getfield.rs
+++ b/datafusion/functions/src/core/getfield.rs
@@ -26,6 +26,7 @@ use datafusion_common::{
use datafusion_expr::{ColumnarValue, Expr, ExprSchemable};
use datafusion_expr::{ScalarUDFImpl, Signature, Volatility};
use std::any::Any;
+use std::sync::Arc;
#[derive(Debug)]
pub struct GetFieldFunc {
@@ -151,7 +152,7 @@ impl ScalarUDFImpl for GetFieldFunc {
}
let arrays = ColumnarValue::values_to_arrays(args)?;
- let array = arrays[0].clone();
+ let array = Arc::clone(&arrays[0]);
let name = match &args[1] {
ColumnarValue::Scalar(name) => name,
@@ -199,7 +200,7 @@ impl ScalarUDFImpl for GetFieldFunc {
let as_struct_array = as_struct_array(&array)?;
match as_struct_array.column_by_name(k) {
None => exec_err!("get indexed field {k} not found in
struct"),
- Some(col) => Ok(ColumnarValue::Array(col.clone())),
+ Some(col) => Ok(ColumnarValue::Array(Arc::clone(col))),
}
}
(DataType::Struct(_), name) => exec_err!(
diff --git a/datafusion/functions/src/core/map.rs
b/datafusion/functions/src/core/map.rs
index 6626831c80..1834c7ac60 100644
--- a/datafusion/functions/src/core/map.rs
+++ b/datafusion/functions/src/core/map.rs
@@ -93,9 +93,9 @@ fn make_map_batch(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
fn get_first_array_ref(columnar_value: &ColumnarValue) -> Result<ArrayRef> {
match columnar_value {
ColumnarValue::Scalar(value) => match value {
- ScalarValue::List(array) => Ok(array.value(0).clone()),
- ScalarValue::LargeList(array) => Ok(array.value(0).clone()),
- ScalarValue::FixedSizeList(array) => Ok(array.value(0).clone()),
+ ScalarValue::List(array) => Ok(array.value(0)),
+ ScalarValue::LargeList(array) => Ok(array.value(0)),
+ ScalarValue::FixedSizeList(array) => Ok(array.value(0)),
_ => exec_err!("Expected array, got {:?}", value),
},
ColumnarValue::Array(array) => exec_err!("Expected scalar, got {:?}",
array),
diff --git a/datafusion/functions/src/core/nvl.rs
b/datafusion/functions/src/core/nvl.rs
index 05515c6e92..a09224acef 100644
--- a/datafusion/functions/src/core/nvl.rs
+++ b/datafusion/functions/src/core/nvl.rs
@@ -21,6 +21,7 @@ use arrow::compute::kernels::zip::zip;
use arrow::datatypes::DataType;
use datafusion_common::{internal_err, Result};
use datafusion_expr::{ColumnarValue, ScalarUDFImpl, Signature, Volatility};
+use std::sync::Arc;
#[derive(Debug)]
pub struct NVLFunc {
@@ -101,13 +102,13 @@ fn nvl_func(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
}
let (lhs_array, rhs_array) = match (&args[0], &args[1]) {
(ColumnarValue::Array(lhs), ColumnarValue::Scalar(rhs)) => {
- (lhs.clone(), rhs.to_array_of_size(lhs.len())?)
+ (Arc::clone(lhs), rhs.to_array_of_size(lhs.len())?)
}
(ColumnarValue::Array(lhs), ColumnarValue::Array(rhs)) => {
- (lhs.clone(), rhs.clone())
+ (Arc::clone(lhs), Arc::clone(rhs))
}
(ColumnarValue::Scalar(lhs), ColumnarValue::Array(rhs)) => {
- (lhs.to_array_of_size(rhs.len())?, rhs.clone())
+ (lhs.to_array_of_size(rhs.len())?, Arc::clone(rhs))
}
(ColumnarValue::Scalar(lhs), ColumnarValue::Scalar(rhs)) => {
let mut current_value = lhs;
diff --git a/datafusion/functions/src/core/nvl2.rs
b/datafusion/functions/src/core/nvl2.rs
index 573ac72425..1144dc0fb7 100644
--- a/datafusion/functions/src/core/nvl2.rs
+++ b/datafusion/functions/src/core/nvl2.rs
@@ -24,6 +24,7 @@ use datafusion_expr::{
type_coercion::binary::comparison_coercion, ColumnarValue, ScalarUDFImpl,
Signature,
Volatility,
};
+use std::sync::Arc;
#[derive(Debug)]
pub struct NVL2Func {
@@ -112,7 +113,7 @@ fn nvl2_func(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
.iter()
.map(|arg| match arg {
ColumnarValue::Scalar(scalar) => scalar.to_array_of_size(len),
- ColumnarValue::Array(array) => Ok(array.clone()),
+ ColumnarValue::Array(array) => Ok(Arc::clone(array)),
})
.collect::<Result<Vec<_>>>()?;
let to_apply = is_not_null(&args[0])?;
diff --git a/datafusion/functions/src/core/struct.rs
b/datafusion/functions/src/core/struct.rs
index 9d4b2e4a0b..c3dee8b1cc 100644
--- a/datafusion/functions/src/core/struct.rs
+++ b/datafusion/functions/src/core/struct.rs
@@ -40,7 +40,7 @@ fn array_struct(args: &[ArrayRef]) -> Result<ArrayRef> {
arg.data_type().clone(),
true,
)),
- arg.clone(),
+ Arc::clone(arg),
))
})
.collect::<Result<Vec<_>>>()?;
@@ -121,30 +121,21 @@ mod tests {
as_struct_array(&struc).expect("failed to initialize function
struct");
assert_eq!(
&Int64Array::from(vec![1]),
- result
- .column_by_name("c0")
- .unwrap()
- .clone()
+ Arc::clone(result.column_by_name("c0").unwrap())
.as_any()
.downcast_ref::<Int64Array>()
.unwrap()
);
assert_eq!(
&Int64Array::from(vec![2]),
- result
- .column_by_name("c1")
- .unwrap()
- .clone()
+ Arc::clone(result.column_by_name("c1").unwrap())
.as_any()
.downcast_ref::<Int64Array>()
.unwrap()
);
assert_eq!(
&Int64Array::from(vec![3]),
- result
- .column_by_name("c2")
- .unwrap()
- .clone()
+ Arc::clone(result.column_by_name("c2").unwrap())
.as_any()
.downcast_ref::<Int64Array>()
.unwrap()
diff --git a/datafusion/functions/src/datetime/date_part.rs
b/datafusion/functions/src/datetime/date_part.rs
index 4906cdc960..e1efb4811e 100644
--- a/datafusion/functions/src/datetime/date_part.rs
+++ b/datafusion/functions/src/datetime/date_part.rs
@@ -123,7 +123,7 @@ impl ScalarUDFImpl for DatePartFunc {
let is_scalar = matches!(array, ColumnarValue::Scalar(_));
let array = match array {
- ColumnarValue::Array(array) => array.clone(),
+ ColumnarValue::Array(array) => Arc::clone(array),
ColumnarValue::Scalar(scalar) => scalar.to_array()?,
};
diff --git a/datafusion/functions/src/datetime/to_timestamp.rs
b/datafusion/functions/src/datetime/to_timestamp.rs
index 4cb91447f3..cbb6f37603 100644
--- a/datafusion/functions/src/datetime/to_timestamp.rs
+++ b/datafusion/functions/src/datetime/to_timestamp.rs
@@ -16,6 +16,7 @@
// under the License.
use std::any::Any;
+use std::sync::Arc;
use arrow::datatypes::DataType::Timestamp;
use arrow::datatypes::TimeUnit::{Microsecond, Millisecond, Nanosecond, Second};
@@ -387,7 +388,7 @@ impl ScalarUDFImpl for ToTimestampNanosFunc {
/// the timezone if it exists.
fn return_type_for(arg: &DataType, unit: TimeUnit) -> DataType {
match arg {
- Timestamp(_, Some(tz)) => Timestamp(unit, Some(tz.clone())),
+ Timestamp(_, Some(tz)) => Timestamp(unit, Some(Arc::clone(tz))),
_ => Timestamp(unit, None),
}
}
@@ -794,10 +795,10 @@ mod tests {
Arc::new(sec_builder.finish().with_timezone("UTC")) as ArrayRef;
let arrays = &[
- ColumnarValue::Array(nanos_timestamps.clone()),
- ColumnarValue::Array(millis_timestamps.clone()),
- ColumnarValue::Array(micros_timestamps.clone()),
- ColumnarValue::Array(sec_timestamps.clone()),
+ ColumnarValue::Array(Arc::clone(&nanos_timestamps)),
+ ColumnarValue::Array(Arc::clone(&millis_timestamps)),
+ ColumnarValue::Array(Arc::clone(µs_timestamps)),
+ ColumnarValue::Array(Arc::clone(&sec_timestamps)),
];
for udf in &udfs {
@@ -836,11 +837,11 @@ mod tests {
let i64_timestamps = Arc::new(i64_builder.finish()) as ArrayRef;
let arrays = &[
- ColumnarValue::Array(nanos_timestamps.clone()),
- ColumnarValue::Array(millis_timestamps.clone()),
- ColumnarValue::Array(micros_timestamps.clone()),
- ColumnarValue::Array(sec_timestamps.clone()),
- ColumnarValue::Array(i64_timestamps.clone()),
+ ColumnarValue::Array(Arc::clone(&nanos_timestamps)),
+ ColumnarValue::Array(Arc::clone(&millis_timestamps)),
+ ColumnarValue::Array(Arc::clone(µs_timestamps)),
+ ColumnarValue::Array(Arc::clone(&sec_timestamps)),
+ ColumnarValue::Array(Arc::clone(&i64_timestamps)),
];
for udf in &udfs {
diff --git a/datafusion/functions/src/lib.rs b/datafusion/functions/src/lib.rs
index 433a4f90d9..b1c55c843f 100644
--- a/datafusion/functions/src/lib.rs
+++ b/datafusion/functions/src/lib.rs
@@ -14,6 +14,8 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
+// Make cheap clones clear: https://github.com/apache/datafusion/issues/11143
+#![deny(clippy::clone_on_ref_ptr)]
//! Function packages for [DataFusion].
//!
diff --git a/datafusion/functions/src/math/abs.rs
b/datafusion/functions/src/math/abs.rs
index 6d07b14f86..f7a17f0caf 100644
--- a/datafusion/functions/src/math/abs.rs
+++ b/datafusion/functions/src/math/abs.rs
@@ -91,7 +91,7 @@ fn create_abs_function(input_data_type: &DataType) ->
Result<MathArrayFunction>
| DataType::UInt8
| DataType::UInt16
| DataType::UInt32
- | DataType::UInt64 => Ok(|args: &Vec<ArrayRef>| Ok(args[0].clone())),
+ | DataType::UInt64 => Ok(|args: &Vec<ArrayRef>|
Ok(Arc::clone(&args[0]))),
// Decimal types
DataType::Decimal128(_, _) =>
Ok(make_decimal_abs_function!(Decimal128Array)),
diff --git a/datafusion/functions/src/math/log.rs
b/datafusion/functions/src/math/log.rs
index 0791561539..ea424c1474 100644
--- a/datafusion/functions/src/math/log.rs
+++ b/datafusion/functions/src/math/log.rs
@@ -109,7 +109,7 @@ impl ScalarUDFImpl for LogFunc {
let mut x = &args[0];
if args.len() == 2 {
x = &args[1];
- base = ColumnarValue::Array(args[0].clone());
+ base = ColumnarValue::Array(Arc::clone(&args[0]));
}
// note in f64::log params order is different than in sql. e.g in sql
log(base, x) == f64::log(x, base)
let arr: ArrayRef = match args[0].data_type() {
diff --git a/datafusion/functions/src/math/round.rs
b/datafusion/functions/src/math/round.rs
index 71ab7c1b43..89554a76fe 100644
--- a/datafusion/functions/src/math/round.rs
+++ b/datafusion/functions/src/math/round.rs
@@ -111,7 +111,7 @@ pub fn round(args: &[ArrayRef]) -> Result<ArrayRef> {
let mut decimal_places =
ColumnarValue::Scalar(ScalarValue::Int64(Some(0)));
if args.len() == 2 {
- decimal_places = ColumnarValue::Array(args[1].clone());
+ decimal_places = ColumnarValue::Array(Arc::clone(&args[1]));
}
match args[0].data_type() {
diff --git a/datafusion/functions/src/math/trunc.rs
b/datafusion/functions/src/math/trunc.rs
index f980e58336..3344438454 100644
--- a/datafusion/functions/src/math/trunc.rs
+++ b/datafusion/functions/src/math/trunc.rs
@@ -117,7 +117,7 @@ fn trunc(args: &[ArrayRef]) -> Result<ArrayRef> {
let precision = if args.len() == 1 {
ColumnarValue::Scalar(Int64(Some(0)))
} else {
- ColumnarValue::Array(args[1].clone())
+ ColumnarValue::Array(Arc::clone(&args[1]))
};
match args[0].data_type() {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
