[impala] 02/03: IMPALA-8612: Fix sporadic NPE when dropping an authorized table

[ta...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit e59e9261339a7a353c05548df84ff0d8793b768d
Author: Gabor Kaszab <ga...@cloudera.com>
AuthorDate: Tue Jun 4 15:12:06 2019 +0200

    IMPALA-8612: Fix sporadic NPE when dropping an authorized table
    
    In the analyze() function of DropTableOrViewStmt it's possible that
    serverName_ is not set when analyzer.getTable() throws. As a result
    when the Catalog executes the drop table DDL it runs into a failing
    Precondition check and throws a NullPointerException when updating
    user privileges. Note, to run into the NPE it's required to have
    authorization enabled.
    
    Change-Id: I70bd7ca4796b24920ee156436bf8bbc682e7d952
    Reviewed-on: http://gerrit.cloudera.org:8080/13508
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../org/apache/impala/analysis/DropTableOrViewStmt.java     | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java b/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
index 3619d9e..be083a4 100644
--- a/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
@@ -28,6 +28,8 @@ import org.apache.impala.thrift.TAccessEvent;
 import org.apache.impala.thrift.TCatalogObjectType;
 import org.apache.impala.thrift.TDropTableOrViewParams;
 import org.apache.impala.thrift.TTableName;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Preconditions;
 
@@ -35,6 +37,8 @@ import com.google.common.base.Preconditions;
  * Represents a DROP TABLE/VIEW [IF EXISTS] statement
  */
 public class DropTableOrViewStmt extends StatementBase {
+  private static final Logger LOG = LoggerFactory.getLogger(DropTableOrViewStmt.class);
+
   protected final TableName tableName_;
   protected final boolean ifExists_;
 
@@ -99,6 +103,9 @@ public class DropTableOrViewStmt extends StatementBase {
   @Override
   public void analyze(Analyzer analyzer) throws AnalysisException {
     dbName_ = analyzer.getTargetDbName(tableName_);
+    // Set the servername here if authorization is enabled because analyzer_ is not
+    // available in the toThrift() method.
+    serverName_ = analyzer.getServerName();
     try {
       FeTable table = analyzer.getTable(tableName_, /* add access event */ true,
           /* add column-level privilege */ false, Privilege.DROP);
@@ -112,10 +119,6 @@ public class DropTableOrViewStmt extends StatementBase {
         throw new AnalysisException(String.format(
             "DROP VIEW not allowed on a table: %s.%s", dbName_, getTbl()));
       }
-      // Set the servername here if authorization is enabled because analyzer_ is not
-      // available in the toThrift() method.
-      serverName_ = analyzer.getServerName();
-
     } catch (TableLoadingException e) {
       // We should still try to DROP tables that failed to load, so that tables that are
       // in a bad state, eg. deleted externally from Kudu, can be dropped.
@@ -125,8 +128,10 @@ public class DropTableOrViewStmt extends StatementBase {
       analyzer.addAccessEvent(new TAccessEvent(
           analyzer.getFqTableName(tableName_).toString(), TCatalogObjectType.TABLE,
           Privilege.DROP.toString()));
+      LOG.info("Ignoring TableLoadingException for {}", tableName_);
     } catch (AnalysisException e) {
       if (!ifExists_) throw e;
+      LOG.info("Ignoring AnalysisException for {}", tableName_);
     }
   }