impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Huaisi Xu (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-1702: Enforce table level consistency
Date Fri, 07 Oct 2016 00:40:13 GMT
Huaisi Xu has posted comments on this change.

Change subject: IMPALA-1702: Enforce table level consistency

Patch Set 10:

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.
File fe/src/main/java/org/apache/impala/analysis/

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.
File fe/src/main/java/org/apache/impala/analysis/

PS9, Line 187: 
             :       table.load(true, client.getHiveClient(), msTbl);
             :       insertStmt_.setTargetTable(table);
> update this as per new design.
File fe/src/main/java/org/apache/impala/analysis/

PS9, Line 154: /**
             :    * Connect tupleDescriptors to tableDescriptors with unique table ids and
             :    * this DescriptorTable ready to be sent to backend.
             :    */
> How about we prefix this comment with a brief description of what this meth

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.
File fe/src/test/java/org/apache/impala/planner/

Line 37: import org.apache.kudu.client.KuduClient;
> Move it below to org.apache.impala group.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Bharath Vissapragada <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Huaisi Xu <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message