Henry Robinson has posted comments on this change. Change subject: OPSAPS-32457: Fix CatalogService Thrift changes to be backwards compatible. ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/2455/3/be/src/service/impala-server.cc File be/src/service/impala-server.cc: Line 1398: Status(err.str() use TStatusCode::INTERNAL_ERROR ? http://gerrit.cloudera.org:8080/#/c/2455/3/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: Line 299: either exclusively uses the single updated/removed field : // or the corresponding list versions of the fields, but not a mix. I think the check doesn't handle the case where neither are set. You don't have to handle it (although it might be worthwhile), but this comment makes it sounds like it does check that condition. Line 895: Distinguish which result field to set based on the number of added : // functions for backwards compatibility. For example, BDR relies : // on a stable catalog Thrift API. Is it always the case that the new use of the API will return > 1 function? -- To view, visit http://gerrit.cloudera.org:8080/2455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec04d07c48d7159d2837667d7039046de126a3ad Gerrit-PatchSet: 3 Gerrit-Project: Impala Gerrit-Branch: cdh5-2.5.0_5.7.0 Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Juan Yu <[email protected]> Gerrit-HasComments: Yes
