Alex Behm has posted comments on this change.

Change subject: IMPALA-1654: General partition exprs in DDL operations.
......................................................................


Patch Set 14:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3942/14/fe/src/main/java/com/cloudera/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/ComputeStatsStmt.java:

Line 251:     table_ = analyzer.getTable(tableName_, Privilege.ALTER);
This is an example that can be cleaned similar to what I did in 
AlterTableStmt.java.

The current code has a strange pattern of first doing analyzer.getTable() and 
then maybe analyzing a TableRef. 
This could be problematic because there is a slight chance these two return 
different tables. It also seems like duplicate work/code.

I think we should just analyze a TableRef at the beginning, just like we now do 
in AnalyzeTableStmt.

Amos, can you please go through the other statements changed in this patch and 
apply that cleanup as well? Thanks!


-- 
To view, visit http://gerrit.cloudera.org:8080/3942
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f
Gerrit-PatchSet: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird <amosb...@gmail.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Amos Bird <amosb...@gmail.com>
Gerrit-Reviewer: Jim Apple <jbap...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to