impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Date Fri, 07 Oct 2016 00:55:50 GMT
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 <hxu@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Huaisi Xu <hxu@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message