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 <[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
