Todd Lipcon has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6846/5/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java:

Line 266:       // We have UnsafeByteOperations, but that only supports an 
entire array. Here
this actually doesn't appear to be true -- UnsafeByteOperations supports 
unsafeWrap() with an offset and length. Still should be fine to do in a 
follow-up patch but at least the TODO can be made more accurate


http://gerrit.cloudera.org:8080/#/c/6846/5/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:

Line 326:       bytes = value;
don't we also need to check whether length == value.length?

maybe this isn't really worth it after all since this seems to only be used 
when deserializing schemas, which isn't that common an operation in the Java 
client (sorry for leading you astray)


-- 
To view, visit http://gerrit.cloudera.org:8080/6846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to