separated redundant code and made implementation generic to non native tables

Author: ragupta

iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java

@@ -2075,6 +2075,13 @@ public boolean isPartitioned(org.apache.hadoop.hive.ql.metadata.Table hmsTable)
     return table.spec().isPartitioned();
   }
 
+  @Override
+  public boolean isPartitionPresent(org.apache.hadoop.hive.ql.metadata.Table table,
+                                    Map<String, String> partitionSpec) throws SemanticException {
+    return getPartitionKeys(table).size() == partitionSpec.size() &&
+            !getPartitions(table, partitionSpec, false).isEmpty();
+  }
+
   @Override
   public Partition getPartition(org.apache.hadoop.hive.ql.metadata.Table table,
       Map<String, String> partitionSpec, RewritePolicy policy) throws SemanticException {

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/DescTableAnalyzer.java

@@ -18,11 +18,9 @@
 
 package org.apache.hadoop.hive.ql.ddl.table.info.desc;
 
-import java.util.List;
 import java.util.Map;
 
 import org.apache.hadoop.hive.common.TableName;
-import org.apache.hadoop.hive.metastore.api.FieldSchema;
 import org.apache.hadoop.hive.ql.ErrorMsg;
 import org.apache.hadoop.hive.ql.QueryState;
 import org.apache.hadoop.hive.ql.ddl.DDLWork;
@@ -162,8 +160,6 @@ private Map<String, String> getPartitionSpec(Hive db, ASTNode node, TableName ta
 
       Map<String, String> partitionSpec = null;
       try {
-        partitionSpec = getPartSpec(partNode);
-        validateUnsupportedPartitionClause(tab, partitionSpec != null && !partitionSpec.isEmpty());
         partitionSpec = getValidatedPartSpec(tab, partNode, db.getConf(), false);
       } catch (SemanticException e) {
         // get exception in resolving partition it could be DESCRIBE table key
@@ -172,16 +168,18 @@ private Map<String, String> getPartitionSpec(Hive db, ASTNode node, TableName ta
       }
 
       if (partitionSpec != null) {
-        Partition part = null;
+        boolean isPartitionPresent;
         try {
-          part = getPartition(tab, partitionSpec);
+          isPartitionPresent = tab.isNonNative() ?
+                  tab.getStorageHandler().isPartitionPresent(tab, partitionSpec) :
+                  db.getPartition(tab, partitionSpec) != null;
         } catch (HiveException e) {
           // if get exception in finding partition it could be DESCRIBE table key
           // return null, continue processing for DESCRIBE table key
           return null;
         }
 
-        if (part == null) {
+        if (!isPartitionPresent) {
           throw new SemanticException(ErrorMsg.INVALID_PARTITION.getMsg(partitionSpec.toString()));
         }
 
@@ -190,20 +188,4 @@ private Map<String, String> getPartitionSpec(Hive db, ASTNode node, TableName ta
     }
     return null;
   }
-
-  private Partition getPartition(Table tab, Map<String, String> partitionSpec) throws HiveException {
-    boolean isIcebergTable = DDLUtils.isIcebergTable(tab);
-    if (isIcebergTable) {
-      List<FieldSchema> partKeys = tab.getStorageHandler().getPartitionKeys(tab);
-      if (partKeys.size() != partitionSpec.size()) {
-        return null;
-      }
-      List<Partition> partList = tab.getStorageHandler().getPartitions(tab, partitionSpec, false);
-      if (partList.isEmpty()) {
-        return null;
-      }
-      return partList.getFirst();
-    }
-    return db.getPartition(tab, partitionSpec, false);
-  }
 }

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/DescTableOperation.java

@@ -40,7 +40,6 @@
 import org.apache.hadoop.hive.metastore.api.MetaException;
 import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
 import org.apache.hadoop.hive.ql.ddl.DDLOperationContext;
-import org.apache.hadoop.hive.ql.ddl.DDLUtils;
 import org.apache.hadoop.hive.ql.ddl.ShowUtils;
 import org.apache.hadoop.hive.ql.ddl.table.info.desc.formatter.DescTableFormatter;
 import org.apache.hadoop.hive.ql.exec.ColumnInfo;
@@ -128,14 +127,7 @@ private Table getTable() throws HiveException {
   private Partition getPartition(Table table) throws HiveException {
     Partition part = null;
     if (desc.getPartitionSpec() != null) {
-      boolean isIcebergTable = DDLUtils.isIcebergTable(table);
-      if (isIcebergTable) {
-        List<Partition> partList = table.getStorageHandler().getPartitions(table, desc.getPartitionSpec(), false);
-        part = (partList.isEmpty()) ? null : partList.getFirst();
-
-      } else {
-        part = context.getDb().getPartition(table, desc.getPartitionSpec(), false);
-      }
+      part = context.getDb().getPartition(table, desc.getPartitionSpec());
       if (part == null) {
         throw new HiveException(ErrorMsg.INVALID_PARTITION,
             StringUtils.join(desc.getPartitionSpec().keySet(), ','), desc.getDbTableName());

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/formatter/TextDescTableFormatter.java

@@ -35,7 +35,6 @@
 import org.apache.hadoop.hive.metastore.api.SourceTable;
 import org.apache.hadoop.hive.metastore.api.StorageDescriptor;
 import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
-import org.apache.hadoop.hive.ql.ddl.DDLUtils;
 import org.apache.hadoop.hive.ql.ddl.ShowUtils;
 import org.apache.hadoop.hive.ql.ddl.ShowUtils.TextMetaDataTable;
 import org.apache.hadoop.hive.ql.ddl.table.info.desc.DescTableDesc;
@@ -172,10 +171,11 @@ private void addPartitionData(DataOutputStream out, HiveConf conf, String column
       boolean isFormatted, boolean isOutputPadded) throws IOException {
     String partitionData = "";
     if (columnPath == null) {
-      boolean isIcebergTable = DDLUtils.isIcebergTable(table);
       List<FieldSchema> partitionColumns = null;
       if (table.isPartitioned()) {
-        partitionColumns = isIcebergTable ? table.getStorageHandler().getPartitionKeys(table) : table.getPartCols();
+        partitionColumns = table.isNonNative() ?
+                table.getStorageHandler().getPartitionKeys(table) :
+                table.getPartCols();
       }
       if (CollectionUtils.isNotEmpty(partitionColumns) &&
           conf.getBoolVar(ConfVars.HIVE_DISPLAY_PARTITION_COLUMNS_SEPARATELY)) {

ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java

@@ -987,6 +987,11 @@ default boolean isPartitioned(org.apache.hadoop.hive.ql.metadata.Table table) {
     return false;
   }
 
+  default boolean isPartitionPresent(org.apache.hadoop.hive.ql.metadata.Table table,
+                                     Map<String, String> partitionSpec) throws SemanticException {
+    return false;
+  }
+
   default boolean hasUndergonePartitionEvolution(org.apache.hadoop.hive.ql.metadata.Table table) {
     throw new UnsupportedOperationException("Storage handler does not support checking if table " +
         "undergone partition evolution.");

ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java

@@ -68,7 +68,6 @@
 import org.apache.hadoop.hive.ql.QueryState;
 import org.apache.hadoop.hive.ql.cache.results.CacheUsage;
 import org.apache.hadoop.hive.ql.ddl.DDLDesc.DDLDescWithWriteId;
-import org.apache.hadoop.hive.ql.ddl.DDLUtils;
 import org.apache.hadoop.hive.ql.ddl.table.constraint.ConstraintsUtils;
 import org.apache.hadoop.hive.ql.exec.FetchTask;
 import org.apache.hadoop.hive.ql.exec.Task;
@@ -1740,23 +1739,6 @@ public static void validatePartSpec(Table tbl, Map<String, String> partSpec,
     }
   }
 
-  /**
-   * Throws an UnsupportedOperationException in case the query has a partition clause but the table is never partitioned
-   * on the HMS-level. Even though table is not partitioned from the HMS's point of view, it might have some other
-   * notion of partitioning under the hood (e.g. Iceberg tables). In these cases, we might decide to proactively throw a
-   * more descriptive, unified error message instead of failing on some other semantic analysis validation step, which
-   * could provide a more counter-intuitive exception message.
-   *
-   * @param tbl The table object, should not be null.
-   * @param partitionClausePresent Whether a partition clause is present in the query (e.g. PARTITION(last_name='Don'))
-   */
-  protected static void validateUnsupportedPartitionClause(Table tbl, boolean partitionClausePresent) {
-    if (partitionClausePresent && tbl.hasNonNativePartitionSupport() && !DDLUtils.isIcebergTable(tbl)) {
-      throw new UnsupportedOperationException("Using partition spec in query is unsupported for non-native table" +
-          " backed by: " + tbl.getStorageHandler().toString());
-    }
-  }
-
   public static void validatePartColumnType(Table tbl, Map<String, String> partSpec,
       ASTNode astNode, HiveConf conf) throws SemanticException {
     if (!HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_TYPE_CHECK_ON_INSERT)) {