Bankim Bhavsar 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 Done 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 That wasn't the intent. Had that been the case, there would have been validation "len <= buf.length" which is missing. 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. > 1. What's the "generated UUID" you're alluding to? This is the UUID that was once generated in Impala. 2nd line taken straight from Impala source. 1st line mentions about the import from Impala. > 2. Is it important that this value matches the value in Apache Impala? If so, > why? Yes. Impala will use the direct BlockBloomFilter API and insert hash value directly using it's own existing implementation of the fast hash. Hence the value must match with Impala. https://github.com/apache/impala/blob/master/be/src/runtime/raw-value-ir.cc#L179 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 Ack -- 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: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Mon, 30 Mar 2020 22:41:17 +0000 Gerrit-HasComments: Yes
