impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zach Amsden (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Date Tue, 07 Mar 2017 00:04:37 GMT
Zach Amsden has posted comments on this change.

Change subject: IMPALA-4318:  Kudu support for CREATE EXTERNAL TABLE AS SELECT
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6261/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:

PS1, Line 210: !createAsSelect
> how about instead of this, checking if there were columns specified or not?
Done


PS1, Line 211:       analyzeExternalKuduTableParams(analyzer);
             :     } else {
             :       analyzeManagedKuduTableParams(analyzer);
> It might be easier to understand if we rebrand these functions now, e.g. an
yeah, doing that.


http://gerrit.cloudera.org:8080/#/c/6261/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:

PS2, Line 57:   // Is this a CREATE TABLE ... AS SELECT statment
            :   private boolean createAsSelect_ = false;
> This just doesn't feel right to me, especially given that this is only used
The "best" thing to do is capture state about whether the Table parameters have defined any
columns.  We already have that state, and use it in the backend, so I think I will just kill
this.


http://gerrit.cloudera.org:8080/#/c/6261/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

PS2, Line 1649: 
> why remove this?
misread the precondition as isExternalTable, I'll add it back


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <zamsden@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Zach Amsden <zamsden@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message