gabotechs commented on code in PR #19379:
URL: https://github.com/apache/datafusion/pull/19379#discussion_r2632020753
##########
datafusion/physical-plan/src/joins/hash_join/partitioned_hash_eval.rs:
##########
@@ -206,13 +259,15 @@ impl Hash for HashTableLookupExpr {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.hash_expr.dyn_hash(state);
self.description.hash(state);
+ Arc::as_ptr(&self.hash_map).hash(state);
}
}
impl PartialEq for HashTableLookupExpr {
fn eq(&self, other: &Self) -> bool {
- Arc::ptr_eq(&self.hash_expr, &other.hash_expr)
+ self.hash_expr.as_ref() == other.hash_expr.as_ref()
&& self.description == other.description
+ && Arc::ptr_eq(&self.hash_map, &other.hash_map)
Review Comment:
it seems like it was like this already, but want to make the question
anyways:
How it is that for two `HashTableLookupExpr` to be equal, the pointer
address of `hash_map` need to be the same? Shouldn't it be up to the `trait
JoinHashMapType` implementation to decide how to perform the equality
comparison?
##########
datafusion/physical-plan/src/joins/hash_join/partitioned_hash_eval.rs:
##########
@@ -206,13 +259,15 @@ impl Hash for HashTableLookupExpr {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.hash_expr.dyn_hash(state);
self.description.hash(state);
+ Arc::as_ptr(&self.hash_map).hash(state);
Review Comment:
Why is hashing the pointer address necessary here? it looks exotic enough to
justify a comment.
--
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]
