Huaisi Xu 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/9//COMMIT_MSG Commit Message: PS9, Line 7: Enforce table level consistency > IMO, this is a little misleading as its sounds similar to consistency guara I think this is just a high level description. It hides implementation details. This is the goal we are trying to achieve. and it contains two things. http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: Line 295: private final HashMap<TableName, Table> referencedTables_ = Maps.newHashMap(); > Thanks for the explanation Alex. Makes sense to me. Although this is not strictly necessary, this is more of completing the circle. http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java: PS9, Line 187: : table.load(true, client.getHiveClient(), msTbl); : insertStmt_.setTargetTable(table); > update this as per new design. Done http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java File fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java: PS9, Line 154: /** : * Connect tupleDescriptors to tableDescriptors with unique table ids and get : * this DescriptorTable ready to be sent to backend. : */ > How about we prefix this comment with a brief description of what this meth Done Line 164: > Sorry, I see what you mean now. Yes, that would work as well, but we'd need This is only for table validation so same table references have the same instance. If this is for validation purposes then it makes more sense to isolate this from anything we used in the code. http://gerrit.cloudera.org:8080/#/c/4349/9/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java: Line 37: import org.apache.kudu.client.KuduClient; > Move it below to org.apache.impala group. Done -- 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
