Alex Behm has posted comments on this change. Change subject: IMPALA-1702: Enforce table level consistency ......................................................................
Patch Set 10: (6 comments) http://gerrit.cloudera.org:8080/#/c/4349/10/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java: Line 155: * Connect tupleDescriptors to tableDescriptors with unique table ids and get Returns the thrift representation of this DescriptorTable. Assigns unique ids to all distinct tables and sets them in tuple descriptors as necessary. http://gerrit.cloudera.org:8080/#/c/4349/10/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java: Line 418: * Our single query table consistency guarantee: Way too elaborate this comment. just say: Validates that all tables in the descriptor table of 'request' have a unique id. Line 422: * 2. All tables reference to the same version of table in Analyzer::getTable(). Remove this block Line 426: private void validateTableConsistency(TExecRequest request) { validateTableIds Line 433: for (TTableDescriptor tableDesc: descTbl.tableDescriptors) { This code seems pretty elaborate for checking unique ids. Wouldn't it be sufficient to stick the ids into a Set and check whether an id already exists? Line 437: throw new IllegalStateException("Failed to verify table consistency for" + We're not validating table consistency. Error report should be consistent with the new validateTableIds() method name. -- To view, visit http://gerrit.cloudera.org:8080/4349 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Huaisi Xu <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
