Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/15511 )
Change subject: WIP IMPALA-9434: Implement Robin Hood Hash Table. ...................................................................... Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/15511/9/be/src/exec/hash-table.inline.h File be/src/exec/hash-table.inline.h: http://gerrit.cloudera.org:8080/#/c/15511/9/be/src/exec/hash-table.inline.h@92 PS9, Line 92: return bucket_idx > I understand this is the short-cut case for insert, and returning the index Agree that it becomes too much overloaded just to fit in robin-hood case. Maybe it is better to return the swap target bucket as an additional return parameter next to parameter found? http://gerrit.cloudera.org:8080/#/c/15511/9/be/src/exec/hash-table.inline.h@132 PS9, Line 132: my_distance > Should my_distance be set to the BucketDistance of the new bucket in the ca You are correct, thanks for pointing this bug! Will fix this in my next submission. -- To view, visit http://gerrit.cloudera.org:8080/15511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28eeccd7f9ccae39e31972391f971901bcbfe986 Gerrit-Change-Number: 15511 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: David Rorke <dro...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 02 Apr 2020 02:23:42 +0000 Gerrit-HasComments: Yes