Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-4357: Fix DROP TABLE for externally deleted Kudu tables ......................................................................
Patch Set 1: It was pointed out by Alex that this is a more general problem than just for Kudu, and we would really like to avoid the string matching in the current version, so I explored some other options: 1. We could propagate a TableLoadingException up to DropTableOrViewStmt. The main issue here is that Analyzer.getTable doesn't currently throw TableLoadingExceptions, so to do this we would need to catch that everywhere else getTable is called, which is about 10 places. We could also define a new version of getTable (or repurpose the existing three argument version that is only used in one place) that throws TableLoadingException, minimizing the related changes that are needed. 2. In theory, we don't need to load the table at all to do a DROP TABLE, so we could basically just skip analysis except the auth check. The problem here is that analyzing the table allows us to throw AnalysisExceptions for certain things, eg. a DROP TABLE on a view or vice vera, or a table that doesn't exist. In these cases you would end up with a CatalogException instead, which is not as nice. 3. Automatically remove tables that Kudu can't find from hms. This is somewhat strange behavior, since you could see a table disappear from Impala without having explicitly DROPed it, but this is how we handle inconsistencies in columns between Kudu and hms, so there's precendent. I started playing around with this, and its easy to delete the table from hms when Kudu fails to load it, but I'm not sure how to remove it from Impala's catalog without running INVALIDATE METADATA. This solution also isn't general and only works for Kudu. I don't have a strong opinion, but I'm not a big fan of 3. -- To view, visit http://gerrit.cloudera.org:8080/5144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b41fc3c0e95508ab67f1d420b033b02ec75a5da Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: No
