Review comments fixes

vikramahuja1001 · vikramahuja1001 · commit c750cf5e23c1 · 2025-08-14T16:36:32.000+05:30
diff --git a/hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/DynamicPartitionFileRecordWriterContainer.java b/hcatalog/core/src/main/java/org/apache/hive/hcatalog/mapreduce/DynamicPartitionFileRecordWriterContainer.java
@@ -27,6 +27,7 @@
 
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.api.Table;
 import org.apache.hadoop.hive.ql.ddl.table.partition.PartitionUtils;
 import org.apache.hadoop.hive.serde2.AbstractSerDe;
 import org.apache.hadoop.hive.serde2.SerDeException;
@@ -73,7 +74,7 @@ class DynamicPartitionFileRecordWriterContainer extends FileRecordWriterContaine
    */
   public DynamicPartitionFileRecordWriterContainer(
       RecordWriter<? super WritableComparable<?>, ? super Writable> baseWriter,
-      TaskAttemptContext context, org.apache.hadoop.hive.metastore.api.Table tbl)
+      TaskAttemptContext context, Table tbl)
       throws IOException, InterruptedException {
     super(baseWriter, context);
     maxDynamicPartitions = jobInfo.getMaxDynamicPartitions();
@@ -91,7 +92,7 @@ public DynamicPartitionFileRecordWriterContainer(
     this.dynamicObjectInspectors = new HashMap<String, ObjectInspector>();
     this.dynamicOutputJobInfo = new HashMap<String, OutputJobInfo>();
     this.HIVE_DEFAULT_PARTITION_VALUE = PartitionUtils.getDefaultPartitionName(tbl.getParameters(),
-            HiveConf.getVar(context.getConfiguration(), HiveConf.ConfVars.DEFAULT_PARTITION_NAME));
+        context.getConfiguration());
   }
 
   @Override
diff --git a/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java b/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java
@@ -877,7 +877,7 @@ public DynamicPartitionCtx createDPContext(
     Table table = IcebergTableUtil.getTable(conf, tableDesc.getProperties());
 
     DynamicPartitionCtx dpCtx = new DynamicPartitionCtx(Maps.newLinkedHashMap(),
-        PartitionUtils.getDefaultPartitionName(hmsTable.getParameters(), (HiveConf) conf),
+        PartitionUtils.getDefaultPartitionName(hmsTable.getParameters(), hiveConf),
         hiveConf.getIntVar(ConfVars.DYNAMIC_PARTITION_MAX_PARTS_PER_NODE));
 
     if (table.spec().isPartitioned() &&
diff --git a/parser/src/java/org/apache/hadoop/hive/ql/parse/AlterClauseParser.g b/parser/src/java/org/apache/hadoop/hive/ql/parse/AlterClauseParser.g
@@ -85,6 +85,7 @@ alterTableStatementSuffix
     | alterStatementSuffixRenameBranch
     | alterStatementSuffixReplaceBranch
     | alterStatementSuffixReplaceTag
+    | alterStatementSuffixSetDefaultPartition
     ;
 
 alterTblPartitionStatementSuffix[boolean partition]
@@ -104,7 +105,6 @@ alterTblPartitionStatementSuffix[boolean partition]
   | alterStatementSuffixRenameCol
   | alterStatementSuffixAddCol
   | alterStatementSuffixDropCol
-  | alterStatementSuffixSetDefaultPartition
   | alterStatementSuffixUpdateColumns
   ;
 
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/PartitionUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/PartitionUtils.java
@@ -24,6 +24,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
 import org.apache.hadoop.hive.metastore.api.FieldSchema;
@@ -193,16 +194,12 @@ public static void addTablePartsOutputs(Hive db, Set<WriteEntity> outputs, Table
     }
   }
 
-  public static String getDefaultPartitionName(Map<String, String> tableParams, HiveConf conf) {
-    return getDefaultPartitionName(tableParams, HiveConf.getVar(conf, HiveConf.ConfVars.DEFAULT_PARTITION_NAME));
-  }
-
-  public static String getDefaultPartitionName(Map<String, String> tableParams, String defaultPartitionName) {
+  public static String getDefaultPartitionName(Map<String, String> tableParams, Configuration conf) {
     // Check if the table has an override for the default partition name
     if (tableParams != null && tableParams.containsKey(HiveStringUtils.DEFAULT_PARTITION_NAME)) {
       return tableParams.get(HiveStringUtils.DEFAULT_PARTITION_NAME);
     } else {
-      return defaultPartitionName;
+      return conf.get(HiveConf.ConfVars.DEFAULT_PARTITION_NAME.varname);
     }
   }
 }
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/setdefaultpartition/AlterTableSetDefaultPartitionAnalyzer.java b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/setdefaultpartition/AlterTableSetDefaultPartitionAnalyzer.java
@@ -34,9 +34,9 @@
  * Analyzer for set default partition command.
  */
 @DDLType(types = HiveParser.TOK_ALTERTABLE_SETDEFAULTPARTITION)
-public class AlterTableSetDefaultPartitionAnalyser extends AbstractAlterTableAnalyzer {
+public class AlterTableSetDefaultPartitionAnalyzer extends AbstractAlterTableAnalyzer {
 
-  public AlterTableSetDefaultPartitionAnalyser(QueryState queryState) throws SemanticException {
+  public AlterTableSetDefaultPartitionAnalyzer(QueryState queryState) throws SemanticException {
     super(queryState);
   }
 
@@ -45,7 +45,7 @@ protected void analyzeCommand(TableName tableName, Map<String, String> partition
       throws SemanticException {
     String tableLevelDefaultPartitionName = unescapeSQLString(command.getChild(0).getText());
     AlterTableSetDefaultPartitionDesc desc = new AlterTableSetDefaultPartitionDesc(tableName,
-            tableLevelDefaultPartitionName);
+        tableLevelDefaultPartitionName);
     addInputsOutputsAlterTable(tableName, partitionSpec, desc, desc.getType(), false);
     rootTasks.add(TaskFactory.get(new DDLWork(getInputs(), getOutputs(), desc)));
   }
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/setdefaultpartition/AlterTableSetDefaultPartitionDesc.java b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/setdefaultpartition/AlterTableSetDefaultPartitionDesc.java
@@ -39,12 +39,7 @@ public AlterTableSetDefaultPartitionDesc(TableName tableName, String tableLevelD
   }
 
   @Explain(displayName = "default partition name", explainLevels = { Level.USER, Level.DEFAULT, Level.EXTENDED })
-  public String tableLevelDefaultPartitionName() {
+  public String getTableLevelDefaultPartitionName() {
     return tableLevelDefaultPartitionName;
   }
-
-  @Override
-  public boolean mayNeedWriteId() {
-    return true;
-  }
 }
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/setdefaultpartition/AlterTableSetDefaultPartitionOperation.java b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/setdefaultpartition/AlterTableSetDefaultPartitionOperation.java
@@ -39,7 +39,7 @@ public AlterTableSetDefaultPartitionOperation(DDLOperationContext context, Alter
   protected void doAlteration(Table table, Partition partition) throws HiveException {
     if (table.isPartitioned()) {
       // put the tableLevelDefaultPartitionName in the table property
-      table.getParameters().put(HiveStringUtils.DEFAULT_PARTITION_NAME, desc.tableLevelDefaultPartitionName());
+      table.getParameters().put(HiveStringUtils.DEFAULT_PARTITION_NAME, desc.getTableLevelDefaultPartitionName());
     } else {
       throw new HiveException(ErrorMsg.NON_PARTITIONED_TABLE, table.getTableName());
     }
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java
@@ -322,8 +322,7 @@ protected void initializeOp(Configuration hconf) throws HiveException {
     }
 
     if (conf.getTableMetadata() != null) {
-      defaultPartitionName = PartitionUtils.getDefaultPartitionName(conf.getTableMetadata().getParameters(),
-              HiveConf.getVar(hconf, HiveConf.ConfVars.DEFAULT_PARTITION_NAME));
+      defaultPartitionName = PartitionUtils.getDefaultPartitionName(conf.getTableMetadata().getParameters(), hconf);
     } else {
       // If the table metadata is not available, we cannot determine the default partition name.
       defaultPartitionName = HiveConf.getVar(hconf, HiveConf.ConfVars.DEFAULT_PARTITION_NAME);
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
@@ -1571,7 +1571,7 @@ public List<Partition> dropPartitions(String catName, String dbName, String tblN
         for (Pair<Integer, byte[]> pair : partExprs) {
           byte[] expr = pair.getRight();
           String filter = generateJDOFilter(table, expr, PartitionUtils.getDefaultPartitionName(table.getParameters(),
-                  conf.get(HiveConf.ConfVars.DEFAULT_PARTITION_NAME.varname)));
+              conf));
           List<Partition> partitions = tt.listPartitionsByFilter(filter);
           for (Partition p : partitions) {
             Partition droppedPartition = tt.dropPartition(p.getValues());
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java
@@ -579,7 +579,7 @@ private static CalcitePlanner getAnalyzer(HiveConf conf, Context ctx) throws Sem
    */
   public static Map<Integer, List<ExprNodeGenericFuncDesc>> getFullPartitionSpecs(
       CommonTree ast, Table table, Configuration conf, boolean canGroupExprs) throws SemanticException {
-    String defaultPartitionName = PartitionUtils.getDefaultPartitionName(table.getParameters(), (HiveConf)conf);
+    String defaultPartitionName = PartitionUtils.getDefaultPartitionName(table.getParameters(), conf);
     Map<String, String> colTypes = new HashMap<>();
     List<FieldSchema> partitionKeys = table.hasNonNativePartitionSupport() ?
         table.getStorageHandler().getPartitionKeys(table) : table.getPartitionKeys();
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java b/ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java
@@ -49,7 +49,7 @@ public enum HiveOperation {
       null),
   ALTERTABLE_DROPCOL("ALTERTABLE_DROPCOL", HiveParser.TOK_ALTERTABLE_DROPCOL, new Privilege[]{Privilege.ALTER_METADATA},
           null),
-  ALTERTABLE_SETDEFAULTPARTITION("ALTERTABLE_SETDEFAULTPARTITION", HiveParser.TOK_ALTERTABLE_SETDEFAULTPARTITION, new Privilege[]{Privilege.ALTER_METADATA},
+  ALTERTABLE_SETDEFAULTPARTITION("ALTERTABLE_SETDEFAULTPARTITION", HiveParser.TOK_ALTERTABLE_SETDEFAULTPARTITION, new Privilege[]{Privilege.ALTER_DATA},
           null),
   ALTERTABLE_REPLACECOLS("ALTERTABLE_REPLACECOLS", HiveParser.TOK_ALTERTABLE_REPLACECOLS,
       new Privilege[]{Privilege.ALTER_METADATA}, null),
diff --git a/ql/src/test/queries/clientpositive/alter_table_set_default_partition.q b/ql/src/test/queries/clientpositive/alter_table_set_default_partition.q
@@ -1,4 +1,3 @@
-drop table if exists tbl_orc;
 
 create external table tbl_orc(a int, b string, c int) partitioned by (d int) stored as orc;
 describe formatted tbl_orc;
@@ -18,28 +17,26 @@ dfs -ls ${hiveconf:hive.metastore.warehouse.dir}/tbl_orc/;
 drop table if exists tbl_orc;
 
 
-create external table tbl_orc(a int, b string, c int) partitioned by (d int, e string, f bigint) stored as orc;
-describe formatted tbl_orc;
-INSERT INTO tbl_orc (a, b, c) values(3, 'ccc', 12);
-INSERT INTO tbl_orc (a, b, c, d, e) values(5, 'ddd', 13, 32, 'tgf');
-INSERT INTO tbl_orc (a, b, c, e, f) values(6, 'eee', 14, 'tgh', 23456876);
-show partitions tbl_orc;
-dfs -ls ${hiveconf:hive.metastore.warehouse.dir}/tbl_orc/;
-alter table tbl_orc set default partition to 'default_partition';
-describe formatted tbl_orc;
-select * from tbl_orc;
-select count(*) from tbl_orc;
-show partitions tbl_orc;
-INSERT INTO tbl_orc (a, b, c) values(4, 'ddd', 23);
-select * from tbl_orc;
-select count(*) from tbl_orc;
-show partitions tbl_orc;
-dfs -ls ${hiveconf:hive.metastore.warehouse.dir}/tbl_orc/;
-drop table if exists tbl_orc;
+create external table tbl_orc_2(a int, b string, c int) partitioned by (d int, e string, f bigint) stored as orc;
+describe formatted tbl_orc_2;
+INSERT INTO tbl_orc_2 (a, b, c) values(3, 'ccc', 12);
+INSERT INTO tbl_orc_2 (a, b, c, d, e) values(5, 'ddd', 13, 32, 'tgf');
+INSERT INTO tbl_orc_2 (a, b, c, e, f) values(6, 'eee', 14, 'tgh', 23456876);
+show partitions tbl_orc_2;
+dfs -ls ${hiveconf:hive.metastore.warehouse.dir}/tbl_orc_2/;
+alter table tbl_orc_2 set default partition to 'default_partition';
+describe formatted tbl_orc_2;
+select * from tbl_orc_2;
+select count(*) from tbl_orc_2;
+show partitions tbl_orc_2;
+INSERT INTO tbl_orc_2 (a, b, c) values(4, 'ddd', 23);
+select * from tbl_orc_2;
+select count(*) from tbl_orc_2;
+show partitions tbl_orc_2;
+dfs -ls ${hiveconf:hive.metastore.warehouse.dir}/tbl_orc_2/;
+drop table if exists tbl_orc_2;
 
 
-drop table if exists tbl_orc_multiple;
-drop table if exists tbl_orc_copy_multiple;
 create external table tbl_orc_multiple(a int, b string, c int) partitioned by (d int, e string, f bigint) stored as orc location '${system:test.tmp.dir}/apps/hive/warehouse/test.db/tbl_orc_multiple/';
 describe formatted tbl_orc_multiple;
 INSERT INTO tbl_orc_multiple (a, b, c) values(3, 'ccc', 12);
diff --git a/ql/src/test/queries/clientpositive/msck_repair_9.q b/ql/src/test/queries/clientpositive/msck_repair_9.q
@@ -15,6 +15,7 @@ SHOW PARTITIONS tbl_x;
 CREATE EXTERNAL TABLE tbl_y (id INT, name STRING) PARTITIONED BY (month INT, day INT) stored as ORC location '${system:test.tmp.dir}/apps/hive/warehouse/test.db/tbl_x/';
 alter table tbl_y set default partition to 'ANOTHER_PARTITION';
 
+dfs ${system:test.dfs.mkdir} -p ${system:test.tmp.dir}/apps/hive/warehouse/test.db/tbl_x/month=DEFAULT/day=DEFAULT;
 set hive.msck.path.validation=skip;
 
 MSCK REPAIR TABLE tbl_y;
@@ -31,7 +32,6 @@ SHOW PARTITIONS tbl_y;
 
 set hive.msck.path.validation=ignore;
 
-dfs ${system:test.dfs.mkdir} -p ${system:test.tmp.dir}/apps/hive/warehouse/test.db//tbl_x/month=DEFAULT/day=DEFAULT;
 MSCK REPAIR TABLE tbl_y;
 SHOW PARTITIONS tbl_y;
 
diff --git a/ql/src/test/results/clientpositive/llap/alter_table_set_default_partition.q.out b/ql/src/test/results/clientpositive/llap/alter_table_set_default_partition.q.out
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java

Original file line number	Diff line number	Diff line change
`@@ -85,6 +85,7 @@ alterTableStatementSuffix`
`85`	`85`	`\| alterStatementSuffixRenameBranch`
`86`	`86`	`\| alterStatementSuffixReplaceBranch`
`87`	`87`	`\| alterStatementSuffixReplaceTag`
	`88`	`+ \| alterStatementSuffixSetDefaultPartition`
`88`	`89`	`;`
`89`	`90`
`90`	`91`	`alterTblPartitionStatementSuffix[boolean partition]`
`@@ -104,7 +105,6 @@ alterTblPartitionStatementSuffix[boolean partition]`
`104`	`105`	`\| alterStatementSuffixRenameCol`
`105`	`106`	`\| alterStatementSuffixAddCol`
`106`	`107`	`\| alterStatementSuffixDropCol`
`107`		`- \| alterStatementSuffixSetDefaultPartition`
`108`	`108`	`\| alterStatementSuffixUpdateColumns`
`109`	`109`	`;`
`110`	`110`
Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@`
`24`	`24`	`import java.util.Map;`
`25`	`25`	`import java.util.Set;`
`26`	`26`
	`27`	`+import org.apache.hadoop.conf.Configuration;`
`27`	`28`	`import org.apache.hadoop.hive.conf.HiveConf;`
`28`	`29`	`import org.apache.hadoop.hive.conf.HiveConf.ConfVars;`
`29`	`30`	`import org.apache.hadoop.hive.metastore.api.FieldSchema;`
`@@ -193,16 +194,12 @@ public static void addTablePartsOutputs(Hive db, Set<WriteEntity> outputs, Table`
`193`	`194`	`}`
`194`	`195`	`}`
`195`	`196`
`196`		`- public static String getDefaultPartitionName(Map<String, String> tableParams, HiveConf conf) {`
`197`		`- return getDefaultPartitionName(tableParams, HiveConf.getVar(conf, HiveConf.ConfVars.DEFAULT_PARTITION_NAME));`
`198`		`- }`
`199`		`-`
`200`		`- public static String getDefaultPartitionName(Map<String, String> tableParams, String defaultPartitionName) {`
	`197`	`+ public static String getDefaultPartitionName(Map<String, String> tableParams, Configuration conf) {`
`201`	`198`	`// Check if the table has an override for the default partition name`
`202`	`199`	`if (tableParams != null && tableParams.containsKey(HiveStringUtils.DEFAULT_PARTITION_NAME)) {`
`203`	`200`	`return tableParams.get(HiveStringUtils.DEFAULT_PARTITION_NAME);`
`204`	`201`	`} else {`
`205`		`- return defaultPartitionName;`
	`202`	`+ return conf.get(HiveConf.ConfVars.DEFAULT_PARTITION_NAME.varname);`
`206`	`203`	`}`
`207`	`204`	`}`
`208`	`205`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,12 +39,7 @@ public AlterTableSetDefaultPartitionDesc(TableName tableName, String tableLevelD`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`@Explain(displayName = "default partition name", explainLevels = { Level.USER, Level.DEFAULT, Level.EXTENDED })`
`42`		`- public String tableLevelDefaultPartitionName() {`
	`42`	`+ public String getTableLevelDefaultPartitionName() {`
`43`	`43`	`return tableLevelDefaultPartitionName;`
`44`	`44`	`}`
`45`		`-`
`46`		`- @Override`
`47`		`- public boolean mayNeedWriteId() {`
`48`		`- return true;`
`49`		`- }`
`50`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public AlterTableSetDefaultPartitionOperation(DDLOperationContext context, Alter`
`39`	`39`	`protected void doAlteration(Table table, Partition partition) throws HiveException {`
`40`	`40`	`if (table.isPartitioned()) {`
`41`	`41`	`// put the tableLevelDefaultPartitionName in the table property`
`42`		`- table.getParameters().put(HiveStringUtils.DEFAULT_PARTITION_NAME, desc.tableLevelDefaultPartitionName());`
	`42`	`+ table.getParameters().put(HiveStringUtils.DEFAULT_PARTITION_NAME, desc.getTableLevelDefaultPartitionName());`
`43`	`43`	`} else {`
`44`	`44`	`throw new HiveException(ErrorMsg.NON_PARTITIONED_TABLE, table.getTableName());`
`45`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -322,8 +322,7 @@ protected void initializeOp(Configuration hconf) throws HiveException {`
`322`	`322`	`}`
`323`	`323`
`324`	`324`	`if (conf.getTableMetadata() != null) {`
`325`		`- defaultPartitionName = PartitionUtils.getDefaultPartitionName(conf.getTableMetadata().getParameters(),`
`326`		`- HiveConf.getVar(hconf, HiveConf.ConfVars.DEFAULT_PARTITION_NAME));`
	`325`	`+ defaultPartitionName = PartitionUtils.getDefaultPartitionName(conf.getTableMetadata().getParameters(), hconf);`
`327`	`326`	`} else {`
`328`	`327`	`// If the table metadata is not available, we cannot determine the default partition name.`
`329`	`328`	`defaultPartitionName = HiveConf.getVar(hconf, HiveConf.ConfVars.DEFAULT_PARTITION_NAME);`