Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-4357: Fix DROP TABLE to pass analysis if the table fails to load ......................................................................
Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/5144/3/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java File fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java: Line 105: // we set it as TABLE as VIEW loading is unlikely to fail and even if it does > What do you think about changing this to TCatalogObjectType.TABLE? We can a Done http://gerrit.cloudera.org:8080/#/c/5144/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: Line 1277: } catch (NoSuchObjectException e) { > This doesn't seem quite right. If we don't have IF EXISTS, then we should t I don't quite understand how this is different from what the patch was originally trying to solve - making it possible to drop tables that were deleted externally in Kudu, except that its for tables deleted externally from Hive. IF EXISTS would still apply, but only for whether or not a table exists in Impala's catalog cache, such that any table that is displayed in "SHOW TABLES" can be deleted without "IF EXISTS". This also brings up the point that the patch only works for the original Kudu situation because we've hard-coded IF EXISTS to true for Kudu operations on line 1259 here. I guess the answer to those concerns is that we're intentionally treating Hive and Kudu differently, since HMS is the ground truth of our catalog, though it seems inconsistent to me. Either way, if I don't make this change another change is needed somewhere, as the AnalysisException that you used to get in the "externally deleted from Hive" situation hinted that "invalidate metadata" was the solution to the situation, but with this patch the ImpalaRuntimeException you get doesn't say that, which would seem to be a usability regression. -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
