Matthew Jacobs has posted comments on this change. Change subject: Simplify creating external Kudu tables and add DROP DATABASE CASCADE ......................................................................
Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/2617/9/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: Line 1121: The use of DdlDelegates combined with the fact that the catalog is disconnected : // from the source of truth, the Hive metastore, leads to some complications here. : // If 'db' is null, that should mean either the database doesn't really exist or the : // database was created outside of Impala and the user didn't run "invalidate : // metadata". If 'db' is not null, there still may be tables in the metastore that : // would require a DdlDelegate. Another scenario is when the user creates tables : // that reqire a DdlDelegate in Impala, then drops the database in Hive. Ideally the : // user visible result of DROP DATABASE CASCADE would be the same in any case. The : // best solution is probably to move the DdlDelegate functionality into Hive. For : // now Impala will assume that any table not in its cache also doesn't require the : // use of a DdlDelegate. If that assumption isn't correct, the database will still : // be dropped in the metastore but the underlying data would remain. Users can issue : // an REFRESH command to load the database metadata before dropping to ensure : // delegates will be used when needed. > Hmm that seems like a bad practice. The code should make sense on its own, I agree the code should make sense, but (a) this doesn't really explain the code below (the comment on l1113 does), it's pointing out an issue with it that isn't addressed here and (b) this is so much text that it detracts from the overall comprehensibility. The code will make the most sense with a more concise comment about the issue, allowing the reader to dig in further if they want on the ticket. -- 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: 9 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
