This is an automated email from the ASF dual-hosted git repository.
comphead pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new 3b59999d feat: Enable `clippy::clone_on_ref_ptr` on `proto` and
`spark_exprs` crates (#859)
3b59999d is described below
commit 3b59999dd8bfb58da3fd70f8f876f9431df7dd2c
Author: Oleks V <[email protected]>
AuthorDate: Wed Aug 21 11:45:11 2024 -0700
feat: Enable `clippy::clone_on_ref_ptr` on `proto` and `spark_exprs` crates
(#859)
* feat: Enable `clippy::clone_on_ref_ptr` on `proto` and `spark_exprs`
crates
---
native/proto/src/lib.rs | 4 ++++
native/spark-expr/src/cast.rs | 9 +++++++--
native/spark-expr/src/if_expr.rs | 12 ++++++------
native/spark-expr/src/lib.rs | 4 ++++
native/spark-expr/src/regexp.rs | 2 +-
native/spark-expr/src/scalar_funcs.rs | 8 ++++----
native/spark-expr/src/scalar_funcs/hash_expressions.rs | 4 ++--
native/spark-expr/src/structs.rs | 12 ++++++------
native/spark-expr/src/temporal.rs | 14 +++++++-------
9 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/native/proto/src/lib.rs b/native/proto/src/lib.rs
index eaf1253c..266bf62d 100644
--- a/native/proto/src/lib.rs
+++ b/native/proto/src/lib.rs
@@ -15,6 +15,10 @@
// specific language governing permissions and limitations
// under the License.
+// The clippy throws an error if the reference clone not wrapped into
`Arc::clone`
+// The lint makes easier for code reader/reviewer separate references clones
from more heavyweight ones
+#![deny(clippy::clone_on_ref_ptr)]
+
// Include generated modules from .proto files.
#[allow(missing_docs)]
pub mod spark_expression {
diff --git a/native/spark-expr/src/cast.rs b/native/spark-expr/src/cast.rs
index e44b1c9f..d8c03d89 100644
--- a/native/spark-expr/src/cast.rs
+++ b/native/spark-expr/src/cast.rs
@@ -619,7 +619,12 @@ fn cast_array(
let casted_dictionary = DictionaryArray::<Int32Type>::new(
dict_array.keys().clone(),
- cast_array(dict_array.values().clone(), to_type, eval_mode,
timezone)?,
+ cast_array(
+ Arc::clone(dict_array.values()),
+ to_type,
+ eval_mode,
+ timezone,
+ )?,
);
let casted_result = match to_type {
@@ -1393,7 +1398,7 @@ impl PhysicalExpr for Cast {
) -> datafusion_common::Result<Arc<dyn PhysicalExpr>> {
match children.len() {
1 => Ok(Arc::new(Cast::new(
- children[0].clone(),
+ Arc::clone(&children[0]),
self.data_type.clone(),
self.eval_mode,
self.timezone.clone(),
diff --git a/native/spark-expr/src/if_expr.rs b/native/spark-expr/src/if_expr.rs
index a5344140..9a90b727 100644
--- a/native/spark-expr/src/if_expr.rs
+++ b/native/spark-expr/src/if_expr.rs
@@ -60,9 +60,9 @@ impl IfExpr {
false_expr: Arc<dyn PhysicalExpr>,
) -> Self {
Self {
- if_expr: if_expr.clone(),
- true_expr: true_expr.clone(),
- false_expr: false_expr.clone(),
+ if_expr: Arc::clone(&if_expr),
+ true_expr: Arc::clone(&true_expr),
+ false_expr: Arc::clone(&false_expr),
case_expr: Arc::new(
CaseExpr::try_new(None, vec![(if_expr, true_expr)],
Some(false_expr)).unwrap(),
),
@@ -102,9 +102,9 @@ impl PhysicalExpr for IfExpr {
children: Vec<Arc<dyn PhysicalExpr>>,
) -> Result<Arc<dyn PhysicalExpr>> {
Ok(Arc::new(IfExpr::new(
- children[0].clone(),
- children[1].clone(),
- children[2].clone(),
+ Arc::clone(&children[0]),
+ Arc::clone(&children[1]),
+ Arc::clone(&children[2]),
)))
}
diff --git a/native/spark-expr/src/lib.rs b/native/spark-expr/src/lib.rs
index cf7fc872..9fb16f94 100644
--- a/native/spark-expr/src/lib.rs
+++ b/native/spark-expr/src/lib.rs
@@ -15,6 +15,10 @@
// specific language governing permissions and limitations
// under the License.
+// The clippy throws an error if the reference clone not wrapped into
`Arc::clone`
+// The lint makes easier for code reader/reviewer separate references clones
from more heavyweight ones
+#![deny(clippy::clone_on_ref_ptr)]
+
mod cast;
mod error;
mod if_expr;
diff --git a/native/spark-expr/src/regexp.rs b/native/spark-expr/src/regexp.rs
index 2672d754..f5e0d53d 100644
--- a/native/spark-expr/src/regexp.rs
+++ b/native/spark-expr/src/regexp.rs
@@ -157,7 +157,7 @@ impl PhysicalExpr for RLike {
) -> Result<Arc<dyn PhysicalExpr>> {
assert!(children.len() == 1);
Ok(Arc::new(RLike::try_new(
- children[0].clone(),
+ Arc::clone(&children[0]),
&self.pattern_str,
)?))
}
diff --git a/native/spark-expr/src/scalar_funcs.rs
b/native/spark-expr/src/scalar_funcs.rs
index ffd6fd21..f4270220 100644
--- a/native/spark-expr/src/scalar_funcs.rs
+++ b/native/spark-expr/src/scalar_funcs.rs
@@ -356,7 +356,7 @@ pub fn spark_round(
make_decimal_array(array, precision, scale, &f)
}
DataType::Float32 | DataType::Float64 => {
- Ok(ColumnarValue::Array(round(&[array.clone()])?))
+ Ok(ColumnarValue::Array(round(&[Arc::clone(array)])?))
}
dt => exec_err!("Not supported datatype for ROUND: {dt}"),
},
@@ -451,12 +451,12 @@ pub fn spark_decimal_div(
let (p3, s3) = get_precision_scale(data_type);
let (left, right): (ArrayRef, ArrayRef) = match (left, right) {
- (ColumnarValue::Array(l), ColumnarValue::Array(r)) => (l.clone(),
r.clone()),
+ (ColumnarValue::Array(l), ColumnarValue::Array(r)) => (Arc::clone(l),
Arc::clone(r)),
(ColumnarValue::Scalar(l), ColumnarValue::Array(r)) => {
- (l.to_array_of_size(r.len())?, r.clone())
+ (l.to_array_of_size(r.len())?, Arc::clone(r))
}
(ColumnarValue::Array(l), ColumnarValue::Scalar(r)) => {
- (l.clone(), r.to_array_of_size(l.len())?)
+ (Arc::clone(l), r.to_array_of_size(l.len())?)
}
(ColumnarValue::Scalar(l), ColumnarValue::Scalar(r)) =>
(l.to_array()?, r.to_array()?),
};
diff --git a/native/spark-expr/src/scalar_funcs/hash_expressions.rs
b/native/spark-expr/src/scalar_funcs/hash_expressions.rs
index 1a403b9e..e76314f4 100644
--- a/native/spark-expr/src/scalar_funcs/hash_expressions.rs
+++ b/native/spark-expr/src/scalar_funcs/hash_expressions.rs
@@ -44,7 +44,7 @@ pub fn spark_murmur3_hash(args: &[ColumnarValue]) ->
Result<ColumnarValue, DataF
let arrays = args[0..args.len() - 1]
.iter()
.map(|arg| match arg {
- ColumnarValue::Array(array) => array.clone(),
+ ColumnarValue::Array(array) => Arc::clone(array),
ColumnarValue::Scalar(scalar) => {
scalar.clone().to_array_of_size(num_rows).unwrap()
}
@@ -88,7 +88,7 @@ pub fn spark_xxhash64(args: &[ColumnarValue]) ->
Result<ColumnarValue, DataFusio
let arrays = args[0..args.len() - 1]
.iter()
.map(|arg| match arg {
- ColumnarValue::Array(array) => array.clone(),
+ ColumnarValue::Array(array) => Arc::clone(array),
ColumnarValue::Scalar(scalar) => {
scalar.clone().to_array_of_size(num_rows).unwrap()
}
diff --git a/native/spark-expr/src/structs.rs b/native/spark-expr/src/structs.rs
index 9ce3eb83..b3c3737d 100644
--- a/native/spark-expr/src/structs.rs
+++ b/native/spark-expr/src/structs.rs
@@ -145,7 +145,7 @@ impl GetStructField {
fn child_field(&self, input_schema: &Schema) ->
DataFusionResult<Arc<Field>> {
match self.child.data_type(input_schema)? {
- DataType::Struct(fields) => Ok(fields[self.ordinal].clone()),
+ DataType::Struct(fields) => Ok(Arc::clone(&fields[self.ordinal])),
data_type => Err(DataFusionError::Plan(format!(
"Expect struct field, got {:?}",
data_type
@@ -177,12 +177,12 @@ impl PhysicalExpr for GetStructField {
.downcast_ref::<StructArray>()
.expect("A struct is expected");
- Ok(ColumnarValue::Array(
- struct_array.column(self.ordinal).clone(),
- ))
+ Ok(ColumnarValue::Array(Arc::clone(
+ struct_array.column(self.ordinal),
+ )))
}
ColumnarValue::Scalar(ScalarValue::Struct(struct_array)) =>
Ok(ColumnarValue::Array(
- struct_array.column(self.ordinal).clone(),
+ Arc::clone(struct_array.column(self.ordinal)),
)),
value => Err(DataFusionError::Execution(format!(
"Expected a struct array, got {:?}",
@@ -200,7 +200,7 @@ impl PhysicalExpr for GetStructField {
children: Vec<Arc<dyn PhysicalExpr>>,
) -> datafusion_common::Result<Arc<dyn PhysicalExpr>> {
Ok(Arc::new(GetStructField::new(
- children[0].clone(),
+ Arc::clone(&children[0]),
self.ordinal,
)))
}
diff --git a/native/spark-expr/src/temporal.rs
b/native/spark-expr/src/temporal.rs
index 34b71a28..415db607 100644
--- a/native/spark-expr/src/temporal.rs
+++ b/native/spark-expr/src/temporal.rs
@@ -118,7 +118,7 @@ impl PhysicalExpr for HourExpr {
children: Vec<Arc<dyn PhysicalExpr>>,
) -> Result<Arc<dyn PhysicalExpr>, DataFusionError> {
Ok(Arc::new(HourExpr::new(
- children[0].clone(),
+ Arc::clone(&children[0]),
self.timezone.clone(),
)))
}
@@ -212,7 +212,7 @@ impl PhysicalExpr for MinuteExpr {
children: Vec<Arc<dyn PhysicalExpr>>,
) -> Result<Arc<dyn PhysicalExpr>, DataFusionError> {
Ok(Arc::new(MinuteExpr::new(
- children[0].clone(),
+ Arc::clone(&children[0]),
self.timezone.clone(),
)))
}
@@ -306,7 +306,7 @@ impl PhysicalExpr for SecondExpr {
children: Vec<Arc<dyn PhysicalExpr>>,
) -> Result<Arc<dyn PhysicalExpr>, DataFusionError> {
Ok(Arc::new(SecondExpr::new(
- children[0].clone(),
+ Arc::clone(&children[0]),
self.timezone.clone(),
)))
}
@@ -393,8 +393,8 @@ impl PhysicalExpr for DateTruncExpr {
children: Vec<Arc<dyn PhysicalExpr>>,
) -> Result<Arc<dyn PhysicalExpr>, DataFusionError> {
Ok(Arc::new(DateTruncExpr::new(
- children[0].clone(),
- self.format.clone(),
+ Arc::clone(&children[0]),
+ Arc::clone(&self.format),
)))
}
@@ -518,8 +518,8 @@ impl PhysicalExpr for TimestampTruncExpr {
children: Vec<Arc<dyn PhysicalExpr>>,
) -> Result<Arc<dyn PhysicalExpr>, DataFusionError> {
Ok(Arc::new(TimestampTruncExpr::new(
- children[0].clone(),
- self.format.clone(),
+ Arc::clone(&children[0]),
+ Arc::clone(&self.format),
self.timezone.clone(),
)))
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
