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-1654: General partition exprs in DDL operations.
Date Sat, 17 Sep 2016 21:12:33 GMT
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 <amosbird@gmail.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Amos Bird <amosbird@gmail.com>
Gerrit-Reviewer: Jim Apple <jbapple@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message