HIVE-29413: Avoid code duplication by updating getPartCols method for iceberg tables

Author: ramitg254

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/show/ShowColumnsOperation.java

@@ -66,10 +66,7 @@ private List<FieldSchema> getColumnsByPattern() throws HiveException {
 
   private List<FieldSchema> getCols() throws HiveException {
     Table table = context.getDb().getTable(desc.getTableName());
-    List<FieldSchema> allColumns = new ArrayList<>();
-    allColumns.addAll(table.getCols());
-    allColumns.addAll(table.getPartCols());
-    return allColumns;
+    return new ArrayList<>(table.getAllCols());
   }
 
   private Matcher getMatcher() {

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

@@ -174,9 +174,7 @@ private void addPartitionData(DataOutputStream out, HiveConf conf, String column
       List<FieldSchema> partitionColumns = null;
       // TODO (HIVE-29413): Refactor to a generic getPartCols() implementation
       if (table.isPartitioned()) {
-        partitionColumns = table.hasNonNativePartitionSupport() ?
-            table.getStorageHandler().getPartitionKeys(table) :
-            table.getPartCols();
+        partitionColumns = table.getPartCols();
       }
       if (CollectionUtils.isNotEmpty(partitionColumns) &&
           conf.getBoolVar(ConfVars.HIVE_DISPLAY_PARTITION_COLUMNS_SEPARATELY)) {

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/PartitionUtils.java

@@ -23,7 +23,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.Map.Entry;
 
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.conf.HiveConf.ConfVars;

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

@@ -91,9 +91,7 @@ public List<String> getValues() {
     values = new ArrayList<>();
 
     // TODO (HIVE-29413): Refactor to a generic getPartCols() implementation
-    for (FieldSchema fs : table.hasNonNativePartitionSupport()
-        ? table.getStorageHandler().getPartitionKeys(table)
-        : table.getPartCols()) {
+    for (FieldSchema fs : table.getPartCols()) {
       String val = partSpec.get(fs.getName());
       values.add(val);
     }

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

@@ -173,7 +173,8 @@ protected void initialize(Table table,
           // set default if location is not set and this is a physical
           // table partition (not a view partition)
           if (table.getDataLocation() != null) {
-            Path partPath = new Path(table.getDataLocation(), Warehouse.makePartName(table.getPartCols(), tPartition.getValues()));
+            Path partPath = new Path(table.getDataLocation(),
+                Warehouse.makePartName(table.getPartCols(), tPartition.getValues()));
             tPartition.getSd().setLocation(partPath.toString());
           }
         }

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

@@ -595,6 +595,9 @@ public boolean equals(Object obj) {
   }
 
   public List<FieldSchema> getPartCols() {
+    if (hasNonNativePartitionSupport()) {
+      return getStorageHandler().getPartitionKeys(this);
+    }
     List<FieldSchema> partKeys = tTable.getPartitionKeys();
     if (partKeys == null) {
       partKeys = new ArrayList<>();
@@ -610,9 +613,7 @@ public FieldSchema getPartColByName(String colName) {
   }
 
   public List<String> getPartColNames() {
-    List<FieldSchema> partCols = hasNonNativePartitionSupport() ?
-        getStorageHandler().getPartitionKeys(this) : getPartCols();
-    return partCols.stream().map(FieldSchema::getName)
+    return getPartCols().stream().map(FieldSchema::getName)
       .collect(Collectors.toList());
   }
 
@@ -761,10 +762,16 @@ private List<FieldSchema> getColsInternal(boolean forMs) {
    * @return List&lt;FieldSchema&gt;
    */
   public List<FieldSchema> getAllCols() {
-    ArrayList<FieldSchema> f_list = new ArrayList<FieldSchema>();
-    f_list.addAll(getCols());
-    f_list.addAll(getPartCols());
-    return f_list;
+    List<FieldSchema> allCols = new ArrayList<>(getCols());
+    Set<String> colNames = allCols.stream()
+        .map(FieldSchema::getName)
+        .collect(Collectors.toSet());
+    for (FieldSchema col : getPartCols()) {
+      if (!colNames.contains(col.getName())) {
+        allCols.add(col);
+      }
+    }
+    return allCols;
   }
 
   public void setPartCols(List<FieldSchema> partCols) {
@@ -812,7 +819,7 @@ public void setOutputFormatClass(String name) throws HiveException {
   }
 
   public boolean isPartitioned() {
-    return hasNonNativePartitionSupport() ? getStorageHandler().isPartitioned(this) : 
+    return hasNonNativePartitionSupport() ? getStorageHandler().isPartitioned(this) :
         CollectionUtils.isNotEmpty(getPartCols());
   }
 

ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java

@@ -807,8 +807,7 @@ public Object process(Node nd, Stack<Node> stack, NodeProcessorCtx ctx,
         for (FieldNode col : cols) {
           int index = originalOutputColumnNames.indexOf(col.getFieldName());
           Table tab = cppCtx.getParseContext().getViewProjectToTableSchema().get(op);
-          List<FieldSchema> fullFieldList = new ArrayList<FieldSchema>(tab.getCols());
-          fullFieldList.addAll(tab.getPartCols());
+          List<FieldSchema> fullFieldList = new ArrayList<>(tab.getAllCols());
           cppCtx.getParseContext().getColumnAccessInfo()
               .add(tab.getCompleteName(), fullFieldList.get(index).getName());
         }

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

@@ -175,7 +175,8 @@ private void analyzeAcidExport(ASTNode ast, Table exportTable, ASTNode tokRefOrN
     //now generate insert statement
     //insert into newTableName select * from ts <where partition spec>
     StringBuilder rewrittenQueryStr = generateExportQuery(
-            newTable.getPartCols(), tokRefOrNameExportTable, (ASTNode) tokRefOrNameExportTable.parent, newTableName);
+        newTable.getPartCols(),
+        tokRefOrNameExportTable, (ASTNode) tokRefOrNameExportTable.parent, newTableName);
     ReparseResult rr = ParseUtils.parseRewrittenQuery(ctx, rewrittenQueryStr);
     Context rewrittenCtx = rr.rewrittenCtx;
     rewrittenCtx.setIsUpdateDeleteMerge(false); //it's set in parseRewrittenQuery()

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

@@ -24,7 +24,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.api.FieldSchema;

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

@@ -193,8 +193,7 @@ private static CharSequence genPartitionClause(Table tbl, List<TransformSpec> pa
 
 
   private static String getColTypeOf(Table tbl, String partKey) {
-    for (FieldSchema fs : tbl.hasNonNativePartitionSupport() ?
-          tbl.getStorageHandler().getPartitionKeys(tbl) : tbl.getPartitionKeys()) {
+    for (FieldSchema fs : tbl.getPartCols()) {
       if (partKey.equalsIgnoreCase(fs.getName())) {
         return fs.getType().toLowerCase();
       }