adriangb commented on code in PR #19379:
URL: https://github.com/apache/datafusion/pull/19379#discussion_r2632031734
##########
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:
Yes in theory that would be better. But given the actual real world usage I
think just saying "if they're not the same hash table treat them as not being
the same" is good enough. The only case this doesn't cover is where two hash
tables have the same data but given how this is used in HashJoinExec that's not
possible (it would be a major bug!).
--
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]
