impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-2.5.0_5.7.0) OPSAPS-32457: Fix CatalogService Thrift changes to be backwards compatible.
Date Fri, 04 Mar 2016 21:15:09 GMT
Henry Robinson has posted comments on this change.

Change subject: OPSAPS-32457: Fix CatalogService Thrift changes to be backwards compatible.
......................................................................


Patch Set 2:

(5 comments)

It looks as though this patch returns everything to use single objects in the response type,
except for the function case where the caller uses the new 'api'. Is that necessary, or defensive
against other issues of this type?

http://gerrit.cloudera.org:8080/#/c/2455/2//COMMIT_MSG
Commit Message:

Line 10: servies
services


http://gerrit.cloudera.org:8080/#/c/2455/2/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

Line 1383: DCHECK
I don't think this should be a DCHECK - do we want malformed RPCs to bring down debug clusters?
Better to return a response with an explanation.


http://gerrit.cloudera.org:8080/#/c/2455/2/common/thrift/CatalogService.thrift
File common/thrift/CatalogService.thrift:

Line 55: 4: optional CatalogObjects.TCatalogObject updated_catalog_object
I suggest changing the name to updated_catalog_object_DEPRECATED. Won't affect BDR, but we
will be able to immediately see any new code that relies on this.


Line 60: removed_catalog_object
ditto for naming


http://gerrit.cloudera.org:8080/#/c/2455/2/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:

Line 1787: response.result.setUpdated_catalog_object(newTable);
This response isn't related to function creation, right? Why do we need to go back to singletons
(rather than lists) for this case?


-- 
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: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.5.0_5.7.0
Gerrit-Owner: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Juan Yu <jyu@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message