HIVE-29669:EXPLAIN DDL fails on ACID tables with "Cannot find valid write ID list" when HIVE_AUTHORIZATION_ENABLED is enabled

ql/src/java/org/apache/hadoop/hive/ql/exec/DDLPlanUtils.java

@@ -26,6 +26,7 @@
 import com.google.common.collect.Sets;
 import java.util.Comparator;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.common.StatsSetupConst;
 import org.apache.hadoop.hive.metastore.TableType;
 import org.apache.hadoop.hive.metastore.Warehouse;
@@ -239,7 +240,7 @@ public class DDLPlanUtils {
       + TABLE_NAME + "> PARTITION <" + PARTITION_NAME + "> FOR COLUMN <"
       + COLUMN_NAME + "> BUT IT IS NOT SUPPORTED YET. THE BASE64 VALUE FOR THE HISTOGRAM IS <"
       + BASE_64_VALUE + "> ";
-  
+
   /**
    * Returns the create database query for a give database name.
    *
@@ -516,13 +517,14 @@ public String getAlterTableStmtCol(ColumnStatisticsData columnStatisticsData, St
    * Parses the ColumnStatistics for all the columns in a given table and adds the alter table update
    * statistics command for each column.
    *
+   * @param conf
    * @param tbl
    */
-  public List<String> getAlterTableStmtTableStatsColsAll(Table tbl)
+  public List<String> getAlterTableStmtTableStatsColsAll(HiveConf conf, Table tbl)
     throws HiveException {
     List<String> alterTblStmt = new ArrayList<>();
     List<String> accessedColumns = getTableColumnNames(tbl);
-    List<ColumnStatisticsObj> tableColumnStatistics = Hive.get().getTableColumnStatistics(
+    List<ColumnStatisticsObj> tableColumnStatistics = Hive.get(conf).getTableColumnStatistics(
         tbl, accessedColumns, true);
 
     ColumnStatisticsObj[] columnStatisticsObj = tableColumnStatistics.toArray(new ColumnStatisticsObj[0]);
@@ -648,24 +650,24 @@ public String getAlterTableStmtPartitionStatsBasic(Partition pt) {
     return command.render();
   }
 
-  public List<String> getDDLPlanForPartitionWithStats(Table table,
+  public List<String> getDDLPlanForPartitionWithStats(HiveConf conf, Table table,
                                                       Map<String, List<Partition>> tableToPartitionList
   ) throws HiveException {
-    List<String> alterTableStmt = new ArrayList<String>();
+    List<String> alterTableStmt = new ArrayList<>();
     String tableName = table.getTableName();
     for (Partition pt : tableToPartitionList.get(tableName)) {
       alterTableStmt.add(getAlterTableAddPartition(pt));
       alterTableStmt.add(getAlterTableStmtPartitionStatsBasic(pt));
     }
     String databaseName = table.getDbName();
-    List<String> partNames = new ArrayList<String>();
+    List<String> partNames = new ArrayList<>();
     //TODO : Check if only Accessed Column Statistics Can be Retrieved From the HMS.
     List<String> columnNames = getTableColumnNames(table);
     tableToPartitionList.get(tableName).forEach(p -> partNames.add(p.getName()));
     Map<String, List<ColumnStatisticsObj>> partitionColStats =
-      Hive.get().getPartitionColumnStatistics(databaseName,
-        tableName, partNames, columnNames,
-        true);
+        Hive.get(conf).getPartitionColumnStatistics(databaseName,
+            tableName, partNames, columnNames,
+            true);
     Map<String, String> partitionToActualName = new HashMap<>();
     tableToPartitionList.get(tableName).forEach(p -> partitionToActualName.put(p.getName(), getPartitionActualName(p)));
     partitionColStats.keySet().stream().sorted().forEach(partitionName ->

ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java

@@ -475,9 +475,9 @@ public void addStats(Table table,List<String> alterTableStmt ,Map<String, List<P
     PerfLogger perfLogger = PerfLogger.getPerfLogger(conf, false);
     perfLogger.perfLogBegin(ExplainTask.class.getName(), PerfLogger.HIVE_GET_TABLE_COLUMN_STATS);
     if (table.isPartitioned()) {
-      alterTableStmt.addAll(ddlPlanUtils.getDDLPlanForPartitionWithStats(table, tablePartitionsMap));
+      alterTableStmt.addAll(ddlPlanUtils.getDDLPlanForPartitionWithStats(conf, table, tablePartitionsMap));
     } else {
-      alterTableStmt.addAll(ddlPlanUtils.getAlterTableStmtTableStatsColsAll(table));
+      alterTableStmt.addAll(ddlPlanUtils.getAlterTableStmtTableStatsColsAll(conf, table));
     }
     perfLogger.perfLogEnd(ExplainTask.class.getName(), PerfLogger.HIVE_GET_TABLE_COLUMN_STATS);
   }
@@ -489,12 +489,12 @@ public void addExplain(String sql , List<String> explainStmt, DDLPlanUtils ddlPl
 
   public void getDDLPlan(PrintStream out) throws Exception {
     DDLPlanUtils ddlPlanUtils = new DDLPlanUtils();
-    Set<String> createDatabase = new TreeSet<String>();
-    List<String> tableCreateStmt = new LinkedList<String>();
-    List<String> tableBasicDef = new LinkedList<String>();
-    List<String> createViewList = new LinkedList<String>();
-    List<String> alterTableStmt = new LinkedList<String>();
-    List<String> explainStmt = new LinkedList<String>();
+    Set<String> createDatabase = new TreeSet<>();
+    List<String> tableCreateStmt = new LinkedList<>();
+    List<String> tableBasicDef = new LinkedList<>();
+    List<String> createViewList = new LinkedList<>();
+    List<String> alterTableStmt = new LinkedList<>();
+    List<String> explainStmt = new LinkedList<>();
     Map<String, Table> tableMap = new HashMap<>();
     Map<String, List<Partition>> tablePartitionsMap = new HashMap<>();
     for (ReadEntity ent : work.getInputs()) {

ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveMetastoreClientFactoryImpl.java

@@ -40,6 +40,10 @@ public HiveMetastoreClientFactoryImpl(HiveConf conf) {
   public IMetaStoreClient getHiveMetastoreClient() throws HiveAuthzPluginException {
     String errMsg = "Error getting metastore client";
     try {
+      Hive db = Hive.getThreadLocal();
+      if (db != null) {
+        return db.getMSC();
+      }
       return Hive.get(hiveConf, false).getMSC();
     } catch (MetaException e) {
       throw new HiveAuthzPluginException(errMsg, e);

ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java

@@ -118,7 +118,7 @@ protected String getTestDataDir() {
   }
 
   @Override
-  void initHiveConf() {
+  protected void initHiveConf() {
     super.initHiveConf();
     //TestTxnCommandsWithSplitUpdateAndVectorization has the vectorized version
     //of these tests.

ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java

@@ -155,7 +155,7 @@ String getPartitionColumns() {
   public ExpectedException expectedException = ExpectedException.none();
 
   @Override
-  void initHiveConf() {
+  protected void initHiveConf() {
     super.initHiveConf();
     //TestTxnCommands2WithSplitUpdateAndVectorization has the vectorized version
     //of these tests.

ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2WithAbortCleanupUsingCompactionCycle.java

@@ -30,7 +30,7 @@ public TestTxnCommands2WithAbortCleanupUsingCompactionCycle() {
   }
 
   @Override
-  void initHiveConf() {
+  protected void initHiveConf() {
     super.initHiveConf();
     MetastoreConf.setBoolVar(hiveConf, MetastoreConf.ConfVars.COMPACTOR_CLEAN_ABORTS_USING_CLEANER, false);
   }

ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2WithSplitUpdateAndVectorization.java

@@ -34,7 +34,7 @@ public TestTxnCommands2WithSplitUpdateAndVectorization() {
   }
 
   @Override
-  void initHiveConf() {
+  protected void initHiveConf() {
     super.initHiveConf();
     hiveConf.setBoolVar(HiveConf.ConfVars.HIVE_VECTORIZATION_ENABLED, true);
   }

ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsForMmTable.java

@@ -74,7 +74,7 @@ public String toString() {
   }
 
   @Override
-  void initHiveConf() {
+  protected void initHiveConf() {
     super.initHiveConf();
     HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.HIVE_ACID_TRUNCATE_USE_BASE, false);
   }

ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommandsWithSplitUpdateAndVectorization.java

@@ -29,7 +29,7 @@ public TestTxnCommandsWithSplitUpdateAndVectorization() {
   }
 
   @Override
-  void initHiveConf() {
+  protected void initHiveConf() {
     super.initHiveConf();
     hiveConf.setBoolVar(HiveConf.ConfVars.HIVE_VECTORIZATION_ENABLED, true);
   }

ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java

@@ -116,7 +116,7 @@ public void setUp() throws Exception {
       HiveMetaStoreClientWithLocalCache.init(hiveConf);
     }
   }
-  void initHiveConf() {
+  protected void initHiveConf() {
     hiveConf = new HiveConfForTest(this.getClass());
     // Multiple tests requires more than one buckets per write. Use a very small value for grouping size to create
     // multiple mapper instances with FileSinkOperators. The number of buckets are depends on the size of the data

ql/src/test/org/apache/hadoop/hive/ql/exec/TestExplainDdlAcidTable.java

@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.exec;
+
+import java.io.File;
+
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.ql.Driver;
+import org.apache.hadoop.hive.ql.QueryState;
+import org.apache.hadoop.hive.ql.TxnCommandsBaseForTests;
+import org.junit.Test;
+
+public class TestExplainDdlAcidTable extends TxnCommandsBaseForTests {
+
+  private static final String TEST_DATA_DIR = new File(System.getProperty("java.io.tmpdir") +
+      File.separator + TestExplainDdlAcidTable.class.getCanonicalName()
+      + "-" + System.currentTimeMillis()
+  ).getPath().replaceAll("\\\\", "/");
+
+  private static final String EXPLAIN_DDL = "EXPLAIN DDL SELECT * FROM " + Table.ACIDTBL;
+
+  @Override
+  protected void initHiveConf() {
+    super.initHiveConf();
+    HiveConf.setBoolVar(hiveConf, HiveConf.ConfVars.HIVE_IN_TEST, false);
+  }
+
+  @Override
+  protected String getTestDataDir() {
+    return TEST_DATA_DIR;
+  }
+
+  @Test
+  public void testExplainDdlAcidTableUnauthorized() throws Exception {
+    runExplainDdl(hiveConf);
+  }
+
+  /**
+   * {@link DDLPlanUtils} must use the query conf.
+   * HIVE-29330 repoints thread-local {@link org.apache.hadoop.hive.ql.metadata.Hive} conf via
+   * {@link org.apache.hadoop.hive.ql.security.authorization.plugin.HiveMetastoreClientFactoryImpl}
+   */
+  @Test
+  public void testExplainDdlAcidTableAuthorized() throws Exception {
+    HiveConf queryConf = new HiveConf(hiveConf);
+    HiveConf.setBoolVar(queryConf, HiveConf.ConfVars.HIVE_AUTHORIZATION_ENABLED, true);
+    runExplainDdl(queryConf);
+  }
+
+  private void runExplainDdl(HiveConf queryConf) throws Exception {
+    Driver driver = new Driver(new QueryState.Builder().withHiveConf(queryConf).build(), null);
+    driver.setMaxRows(10000);
+    try {
+      driver.run(EXPLAIN_DDL);
+    } finally {
+      driver.close();
+      driver.destroy();
+    }
+  }
+}