[Impala-CR](cdh5-trunk) Simplify creating external Kudu tables and add DROP DATABASE CASCADE

["Matthew Jacobs (Code Review)" <ge...@cloudera.org>]

Matthew Jacobs has posted comments on this change.

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


Patch Set 13:

(5 comments)

Thanks! A few more comments but I'm getting close.

http://gerrit.cloudera.org:8080/#/c/2617/13/fe/src/main/java/com/cloudera/impala/analysis/CreateTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/CreateTableStmt.java:

Line 59: public
you can make this package-visible for the tests since it's also in c.c.i.analysis.


http://gerrit.cloudera.org:8080/#/c/2617/13/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:

Line 1114: The expectation is Impala will always know about any Kudu tables
         :     // because Hive doesn't support Kudu yet.
Hm... but what about the case where you restart your impala cluster? I'd think we'd have null dbs until they're fetched. Maybe Dimitris can comment. Not that we'd change the code, but this statement might not be true. We may have to say a REFRESH is recommended before dropping Kudu tables, especially in any preliminary docs, whenever we do that.


Line 1113: // The db == null case isn't handled. The only tables this should matter for are
         :     // Kudu tables. The expectation is Impala will always know about any Kudu tables
         :     // because Hive doesn't support Kudu yet. When Hive supports Kudu the DDL delegates
         :     // can be removed. https://issues.cloudera.org/browse/IMPALA-3424 tracks the removal.
Thank you! This is very clear.


Line 1530: Look into handling
TODO: Handle "IF NOT EXISTS" for Kudu
(so we can grep for TODOs, and also more direct)


Line 2726: if (params.getLocation() != null) {
         :       sd.setLocation(params.getLocation());
         :     }
1 line


-- 
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: 13
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Casey Ching <ca...@cloudera.com>
Gerrit-Reviewer: Casey Ching <ca...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes