Casey Ching has posted comments on this change.

Change subject: Simplify creating external Kudu tables and add DROP DATABASE 
CASCADE
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/2617/10/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java:

Line 1317:         {"TEXTFILE", "SEQUENCEFILE", "PARQUET", "PARQUETFILE", 
"RCFILE"};
> no kudu?
Right, Kudu tables use "distributed by" instead of "partitioned by".


Line 1537:         "'kudu.key_columns' = 'a,b,c'" +
> general questions:
To find the data types you'd look up the column by name in the column 
definitions. This is a bad test because it relies on incomplete analysis. These 
key columns should really be rejected since they weren't defined in the table 
(there is no column "a").

Yes in the managed table update there will be "primary key" syntax and the 
analysis will be updated/redone.


Line 1654:         "ROW FORMAT cannot be specified for file format KUDU.");
> kudu shouldn't be referred to as a file format (storage manager?)
Are you ok with "...specified for <fileFormat> tables.", to avoid a special 
case for Kudu.


Line 1678:         "A storage handler cannot be specified for a Kudu table.");
> this doesn't need to return a different error message than l1662.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic141102818b6dad3016181b179a14024d0ff709d
Gerrit-PatchSet: 10
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Casey Ching <[email protected]>
Gerrit-Reviewer: Casey Ching <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-HasComments: Yes

Reply via email to