mbutrovich commented on code in PR #2286:
URL: https://github.com/apache/datafusion-comet/pull/2286#discussion_r2353161888


##########
native/spark-expr/src/agg_funcs/covariance.rs:
##########
@@ -38,14 +38,23 @@ use std::sync::Arc;
 /// The implementation mostly is the same as the DataFusion's implementation. 
The reason
 /// we have our own implementation is that DataFusion has UInt64 for 
state_field count,
 /// while Spark has Double for count.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Covariance {
     name: String,
     signature: Signature,
     stats_type: StatsType,
     null_on_divide_by_zero: bool,
 }
 
+impl std::hash::Hash for Covariance {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.name.hash(state);
+        self.signature.hash(state);
+        (self.stats_type as u8).hash(state);

Review Comment:
   StatsType does not implement Hash, so here we do a cast. 
`std::mem::discriminant` is the solution if the enum gets any more complex.



##########
native/spark-expr/src/agg_funcs/covariance.rs:
##########
@@ -38,14 +38,23 @@ use std::sync::Arc;
 /// The implementation mostly is the same as the DataFusion's implementation. 
The reason
 /// we have our own implementation is that DataFusion has UInt64 for 
state_field count,
 /// while Spark has Double for count.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Covariance {
     name: String,
     signature: Signature,
     stats_type: StatsType,
     null_on_divide_by_zero: bool,
 }
 
+impl std::hash::Hash for Covariance {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.name.hash(state);
+        self.signature.hash(state);
+        (self.stats_type as u8).hash(state);

Review Comment:
   `StatsType` does not implement Hash, so here we do a cast. 
`std::mem::discriminant` is the solution if the enum gets any more complex.



-- 
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]

Reply via email to