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 3: (1 comment) The closest JIRA I could find was IMPALA-594, but that's more general than what we're really doing here. Other situations (besides externally dropped from Kudu) that I tested: - Dropping a table that was externally removed from Hive: the previous version of the patch didn't actually work for this situation. I fixed that with the change in CatalogOpExecutor. - Dropping a table when Hive (or Kudu as appropriate) is unreachable, eg. because it was killed. Even with the patch, this still doesn't work. Instead of getting the AnalysisException you would get previously, you now get an ImpalaRuntimeException. I think this is reasonable for now. - Running a DROP that actually should fail analysis, eg. DROP TABLE on a view or vice versa, that now passes analysis due to a transient TableLoadingException, eg. HMS is under load and the loading times out, then actually does go through to execution - we'll get a CatalogException, which is fine. - Dropping a table that fails to load due to unsupported column types, works with this patch. http://gerrit.cloudera.org:8080/#/c/5144/2/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java File fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java: Line 100: } > We need to add an access event for auditing in this case, because we are in The one problem here is that if table loading failed, we don't know if its a table or a view. I put TCatalogObjectType.UNKNOWN, but that seems less than ideal. -- 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: 3 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
