Huaisi Xu has posted comments on this change. Change subject: IMPALA-3499: Split catalog update ......................................................................
Patch Set 14: (5 comments) http://gerrit.cloudera.org:8080/#/c/3067/14/be/src/service/frontend.h File be/src/service/frontend.h: PS14, Line 38: arguement > argument Done PS14, Line 39: contains > No, I believe it should be a singular verb (see http://english.stackexchang ok.. I used to take exams on these. :-) looks like I missed this.. thank you http://gerrit.cloudera.org:8080/#/c/3067/14/be/src/util/jni-util.h File be/src/util/jni-util.h: Line 275: jobjectArray jArrayOfByteArray = > It's the C++ representation of a Java object, and the naming style depends Done http://gerrit.cloudera.org:8080/#/c/3067/14/fe/src/main/java/com/cloudera/impala/service/JniFrontend.java File fe/src/main/java/com/cloudera/impala/service/JniFrontend.java: PS14, Line 164: if (!merged_update_request.isSetIs_delta()) { > isSet* should return true if the field is either a) optional and has been e Thanks. so I think the constructor I called makes this required field irrelevant. since we only use it to merge deserialized update it is fine right? and this looks clearer that we want it to be set at the first time. But I will use another constructor that you suggested. thank you! PS14, Line 171: if (!merged_update_request.isSetUpdated_objects()) { > Huh, that's an annoying implementation detail in the Java thrift libraries. I should have done this.. will do. -- To view, visit http://gerrit.cloudera.org:8080/3067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I176db25124a32944f2396ce8aafbed49cac95928 Gerrit-PatchSet: 14 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-HasComments: Yes
