Alex Behm 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:       return Status(err.str());
> use TStatusCode::INTERNAL_ERROR ?
Done


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 300:     // 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 
Not all responses have updated/removed objects. For example, CREATE TABLE IF 
NOT EXISTS will return success but no updated object if the table already 
exists.

I changed the comment to clarify.


Line 897:       // on a stable catalog Thrift API.
> Is it always the case that the new use of the API will return > 1 function?
No, not always the case. Changed to only use those result fields for persistent 
Java fns.

Made the same change for drop fn.


-- 
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

Reply via email to