alamb commented on code in PR #19910:
URL: https://github.com/apache/datafusion/pull/19910#discussion_r2722478844
##########
datafusion/physical-plan/src/aggregates/group_values/single_group_by/primitive.rs:
##########
@@ -128,7 +128,9 @@ where
let hash = key.hash(state);
let insert = self.map.entry(
hash,
- |&(g, _)| unsafe {
self.values.get_unchecked(g).is_eq(key) },
+ |&(g, h)| unsafe {
+ hash == h &&
self.values.get_unchecked(g).is_eq(key)
Review Comment:
I double checked that Hashbrown says
https://docs.rs/hashbrown/latest/hashbrown/struct.HashTable.html#method.entry
> This method will call eq for all entries with the given hash, but may also
call it for entries with a different hash. eq should only return true for the
desired entry, at which point the search is stopped.
So checking the hash first makes senes to me
--
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]
