Henry Robinson 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 PS14, Line 39: contains > I think after "each" I should use xxx+"s"? No, I believe it should be a singular verb (see http://english.stackexchange.com/questions/12387/each-with-plural-or-singular-verb if you really want to get into it :) 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 = > ok. but for this place I think this is a java object though.. I will It's the C++ representation of a Java object, and the naming style depends on the source language, which is C++ here. 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()) { > I am not sure why "required field" matters here.. could you elaborate? isSet* should return true if the field is either a) optional and has been explicitly set or b) required. It's used to detect if an optional field has been set. The Java implementation looks like it can also return false if a required field hasn't been set after construction, but the serialization methods should always set every field. PS14, Line 171: if (!merged_update_request.isSetUpdated_objects()) { > No I cannot. that is get by TUpdateCatalogCacheRequest merged_update_reques Huh, that's an annoying implementation detail in the Java thrift libraries. Instead, use the constructor to initialise the fields: TUpdateCatalogCacheRequest(false, new TUniqueId(0L, 0L), Lists.newArrayList(), Lists.newArrayList()) -- 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
