Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15600 )
Change subject: [util] Add special handling for nullptr in fast hash ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/15600/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15600/1//COMMIT_MSG@20 PS1, Line 20: compatibilty compatibility http://gerrit.cloudera.org:8080/#/c/15600/1/java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java File java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java: http://gerrit.cloudera.org:8080/#/c/15600/1/java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java@a92 PS1, Line 92: Perhaps the intent was to allow computing the hash on a slice of the array rather than the array in its entirety? It's mostly academic though: if we never actually used it that way, it doesn't matter. http://gerrit.cloudera.org:8080/#/c/15600/1/java/kudu-client/src/main/java/org/apache/kudu/util/HashUtil.java@24 PS1, Line 24: // Constant imported from Apache Impala used to compute hash values for special cases. : // Constant obtained by taking lower bytes of generated UUID. Couple things aren't super clear here: 1. What's the "generated UUID" you're alluding to? 2. Is it important that this value matches the value in Apache Impala? If so, why? http://gerrit.cloudera.org:8080/#/c/15600/1/src/kudu/util/hash_util.h File src/kudu/util/hash_util.h: http://gerrit.cloudera.org:8080/#/c/15600/1/src/kudu/util/hash_util.h@32 PS1, Line 32: // Constant imported from Apache Impala used to compute hash values for special cases. If you end up changing the comment in the Java code to reflect my questions, please update this one too. -- To view, visit http://gerrit.cloudera.org:8080/15600 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf1ccff3dde7ccf54c4c2c6c2910915c69153316 Gerrit-Change-Number: 15600 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 30 Mar 2020 20:24:36 +0000 Gerrit-HasComments: Yes
