HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level configuration #5943

Aggarwal-Raghav · 2025-07-08T16:22:41Z

What changes were proposed in this pull request?

Check HIVE-29066, Ensure that the partitionValidationPattern is picked from the metaconf instead of HMSHandler Object.

Why are the changes needed?

To honour, modified session level metaconf.

Does this PR introduce any user-facing change?

NO

How was this patch tested?

q file is attached.

Aggarwal-Raghav · 2025-07-10T17:41:34Z

The test failure is flaky as reported in https://issues.apache.org/jira/browse/HIVE-29061, will re-trigger if there are any review commets.

Aggarwal-Raghav · 2025-07-10T17:42:03Z

Can you please help with the review @deniskuzZ @okumin ?

...ne-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java

Aggarwal-Raghav · 2025-07-12T15:08:47Z

Also, in HS2 DynamicPartitionCtx#whiteListPattern field exist per object. I am seeing the issue in cluster as well.
Testing (when setting the conf in beeline session)

(when i set the config in hive-site.xml)

Unfortunately, its not happening in q file. Can you suggest should I make similar changes in DynamicPartitionCtx and org/apache/hadoop/hive/ql/exec/FileSinkOperator.java:1129?

Aggarwal-Raghav · 2025-07-12T20:22:18Z

The reason why session level hive.metastore.partition.name.whitelist.pattern is because of this line:

hive/ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicPartitionCtx.java

Line 129 in f2fdea0

    
           confVal = Hive.get().getMetaConf(ConfVars.PARTITION_NAME_WHITELIST_PATTERN.getHiveName());

If we are using hive.metastore.partition.name.whitelist.pattern then it should look in:

confVal = Hive.get().getConf().get(ConfVars.PARTITION_NAME_WHITELIST_PATTERN.getHiveName());

instead of getMetaConf

In q files, as HMS runs is embedded the conf object "might be" shared.

deniskuzZ · 2025-07-24T13:59:43Z

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

@@ -1241,6 +1227,8 @@ public void process(Object row, int tag) throws HiveException {
    } catch (SerDeException e) {
      closeWriters(true);
      throw new HiveException(e);
+    } catch (MetaException ex) {
+      throw new RuntimeException(ex);


should we throw HiveException? maybe squash with above SerDeException

} catch (SerDeException | MetaException e) { closeWriters(true); throw new HiveException(e); }

Doing so can result in error in q file in dynamic_partitions_with_whitelist.q

< Caused by: java.lang.NullPointerException: Cannot invoke "org.apache.hadoop.hive.ql.exec.FileSinkOperator$FSPaths.closeWriters(boolean, java.util.List)" because "this.fpaths" is null --- > Caused by: java.lang.RuntimeException: MetaException(message:Partition value 'val_129' contains a character not matched by whitelist pattern '[^9]*'. Configure with hive.metastore.partition.name.whitelist.pattern for dynamic partitioning otherwise use metaconf:metastore.partition.name.whitelist.pattern) 37a38,39

Earlier it was HiveFatalException, let's contiue that what do you think?

} catch (SerDeException e) { closeWriters(true); throw new HiveException(e); } catch (MetaException e) { throw new HiveFatalException(e); }

CC @deniskuzZ

deniskuzZ · 2025-07-24T14:00:56Z

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

-import org.apache.hadoop.hive.ql.metadata.HiveFatalException;
-import org.apache.hadoop.hive.ql.metadata.HiveStorageHandler;
-import org.apache.hadoop.hive.ql.metadata.HiveUtils;
+import org.apache.hadoop.hive.ql.metadata.*;


please avoid wildcard imports

Apologies, Editor's doing.

deniskuzZ · 2025-07-24T14:05:53Z

...astore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreServerUtils.java

-          "' contains a character " + "not matched by whitelist pattern '" +
-          partitionValidationPattern.toString() + "'.  " + "(configure with " +
-          MetastoreConf.ConfVars.PARTITION_NAME_WHITELIST_PATTERN.getVarname() + ")");
+  public static Pattern getPartitionValidationRegex(Configuration conf) {


can it be private?

deniskuzZ · 2025-07-24T14:07:23Z

...astore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreServerUtils.java

-  public static boolean partitionNameHasValidCharacters(List<String> partVals,
-                                                        Pattern partitionValidationPattern) {
-    return getPartitionValWithInvalidCharacter(partVals, partitionValidationPattern) == null;
+  public static boolean partitionNameHasValidCharacters(List<String> partVals, Configuration conf) {


same, can it be private?

It is referenced in HMSHandler#partition_name_has_valid_characters() but we can make getPartitionValWithInvalidCharacter private.

deniskuzZ · 2025-07-24T14:09:57Z

...astore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreServerUtils.java


-    return null;
+  public static String getPartitionValWithInvalidCharacter(


we should move this code to metastore-common module (MetaStoreUtils), otherwise, we are coupling hive-exec with metastore-server

Ack. Let me check.

@deniskuzZ, I have a question, can you please help me clear a doubt for me?

FileSinkOperator#process() calling MetaStoreServerUtils.validatePartitionNameCharacters (Line 1120) is also coupling right? Yes/No 😅

I believe its Yes, then I'm inclined toward having duplicate methods for partition regex validation in HS2 as earlier it was.

Another question:
As FileSinkOperator runs inside TezTask for dynamic partitioning (insert overwrite query) is that why it is working? as necessary java classes are serialized via kryo to tez task because without thrift, the communication shouldn't be possible b/w ql module and metastore-server? Am I missing something?

Aggarwal-Raghav · 2025-07-29T16:24:13Z

ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicPartitionCtx.java

@@ -91,7 +91,7 @@ public DynamicPartitionCtx(List<String> partColNames, String defaultPartName,
    this.spPath = null;
    String confVal;
    try {
-      confVal = Hive.get().getMetaConf(ConfVars.PARTITION_NAME_WHITELIST_PATTERN.getHiveName());
+      confVal = Hive.get().getMetaConf(ConfVars.PARTITION_NAME_WHITELIST_PATTERN.getVarname());


This change is enough for fixing dynamic partition. Reasoning:

hivename: hive.metastore.partition.name.whitelist.pattern
varname: metastore.partition.name.whitelist.pattern

As users do set metaconf:metastore.partition.name.whitelist.pattern=[^9]*, then varname is used not the hivename. I believe it was never working earlier also for session level.

NOTE: This also means that in hive-site.xml as well this is now going to be picked instead of hivename.

<property> <name>metastore.partition.name.whitelist.pattern</name> <value>[^9]*</value> </property>

This shouldn't be an issue because:

in HiveConf this config is deprecated.

If users still uses hive.metastore.partition.name.whitelist.pattern, then also MoveTask will fail (add_partitions_req) which is indireclty the expected behaviour

Error: Error while compiling statement: FAILED: Execution Error, return code 40000 from org.apache.hadoop.hive.ql.exec.MoveTask. MetaException(message:Partition value '09' contains a character not matched by whitelist pattern '[^9]*'. Configure with metastore.partition.name.whitelist.pattern); Query ID: raghav_20250729215001_82d33e22-04db-4be3-8b01-189504089bf6 (state=08S01,code=40000)

If users uses hive.metastore.partition.name.whitelist.pattern for HMS Side operation like alter then it will also work becuase of MetastoreConf#getVar() which check for hivename as failsafe. That's why q file are not required to change.

Aggarwal-Raghav · 2025-07-29T16:27:00Z

ql/src/test/queries/clientnegative/dynamic_partitions_with_whitelist.q

@@ -17,4 +16,5 @@ load data local inpath '../../data/files/bmj/000000_0' INTO TABLE source_table p
 -- If the directory is not empty the hook will throw an error, instead the error should come from the metastore
 -- This shows that no dynamic partitions were created and left behind or had directories created

+SET metaconf:metastore.partition.name.whitelist.pattern=[^9]*;


The reason for moving this line below is, the metaconf will be resetted after any meta-operation i.e create etc. My assumption is, metconf should always be a line before the config is consumed in the query. the question can be tracked here.

… level configuration

sonarqubecloud · 2025-07-30T05:55:34Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.2% Duplication on New Code

See analysis details on SonarQube Cloud

asf-ci-hive added the tests pending label Jul 8, 2025

Aggarwal-Raghav mentioned this pull request Jul 8, 2025

[DRAFT] HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level metaconf #5938

Closed

asf-ci-hive added tests failed and removed tests pending labels Jul 8, 2025

Aggarwal-Raghav force-pushed the HIVE-29066 branch from bbf81bd to 307e7df Compare July 10, 2025 09:52

asf-ci-hive added tests pending tests unstable and removed tests failed tests pending labels Jul 10, 2025

deniskuzZ reviewed Jul 10, 2025

View reviewed changes

...ne-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java Outdated Show resolved Hide resolved

asf-ci-hive added tests pending and removed tests unstable labels Jul 12, 2025

Aggarwal-Raghav force-pushed the HIVE-29066 branch from 1411a40 to 8e20862 Compare July 12, 2025 19:41

asf-ci-hive added tests unstable tests pending and removed tests pending tests unstable labels Jul 12, 2025

Aggarwal-Raghav changed the title ~~HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level metaconf~~ [WIP] HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level metaconf Jul 13, 2025

Aggarwal-Raghav force-pushed the HIVE-29066 branch from 8e20862 to 3afea9d Compare July 13, 2025 09:53

asf-ci-hive added tests pending tests passed and removed tests unstable tests pending labels Jul 13, 2025

Aggarwal-Raghav changed the title ~~[WIP] HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level metaconf~~ HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level metaconf Jul 13, 2025

Aggarwal-Raghav requested a review from deniskuzZ July 15, 2025 17:30

Aggarwal-Raghav changed the title ~~HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level metaconf~~ HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level configuration Jul 23, 2025

deniskuzZ reviewed Jul 24, 2025

View reviewed changes

Aggarwal-Raghav force-pushed the HIVE-29066 branch from 3afea9d to 3115932 Compare July 29, 2025 16:13

asf-ci-hive added tests pending and removed tests passed labels Jul 29, 2025

Aggarwal-Raghav commented Jul 29, 2025

View reviewed changes

asf-ci-hive added tests unstable and removed tests pending labels Jul 29, 2025

HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session…

9194b78

… level configuration

Aggarwal-Raghav force-pushed the HIVE-29066 branch from 3115932 to 9194b78 Compare July 30, 2025 04:41

asf-ci-hive added tests pending and removed tests unstable labels Jul 30, 2025

asf-ci-hive added tests passed and removed tests pending labels Jul 30, 2025

Aggarwal-Raghav requested a review from deniskuzZ July 30, 2025 17:42


		return null;
		public static String getPartitionValWithInvalidCharacter(

HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level configuration #5943

Are you sure you want to change the base?

HIVE-29066: PARTITION_NAME_WHITELIST_PATTERN is not honouring session level configuration #5943

Conversation

Aggarwal-Raghav commented Jul 8, 2025

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Aggarwal-Raghav commented Jul 10, 2025

Uh oh!

Aggarwal-Raghav commented Jul 10, 2025

Uh oh!

Uh oh!

Aggarwal-Raghav commented Jul 12, 2025

Uh oh!

Aggarwal-Raghav commented Jul 12, 2025

Uh oh!

deniskuzZ Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deniskuzZ Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deniskuzZ Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jul 30, 2025

Quality Gate passed

Uh oh!

Uh oh!

deniskuzZ Jul 24, 2025 •

edited

Loading

deniskuzZ Jul 24, 2025 •

edited

Loading

deniskuzZ Jul 24, 2025 •

edited

Loading