Aggarwal-Raghav commented on code in PR #5943:
URL: https://github.com/apache/hive/pull/5943#discussion_r2231806097
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreServerUtils.java:
##########
@@ -255,31 +253,44 @@ public static double decimalToDouble(Decimal decimal) {
return new BigDecimal(new BigInteger(decimal.getUnscaled()),
decimal.getScale()).doubleValue();
}
- public static void validatePartitionNameCharacters(List<String> partVals,
- Pattern
partitionValidationPattern) throws MetaException {
-
- String invalidPartitionVal = getPartitionValWithInvalidCharacter(partVals,
partitionValidationPattern);
- if (invalidPartitionVal != null) {
- throw new MetaException("Partition value '" + invalidPartitionVal +
- "' 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) {
+ String partitionValidationRegex =
+ MetastoreConf.getVar(conf,
MetastoreConf.ConfVars.PARTITION_NAME_WHITELIST_PATTERN);
+ if (partitionValidationRegex != null &&
!partitionValidationRegex.isEmpty()) {
+ return Pattern.compile(partitionValidationRegex);
}
+ return null;
}
- private static String getPartitionValWithInvalidCharacter(List<String>
partVals,
- Pattern
partitionValidationPattern) {
- if (partitionValidationPattern == null) {
- return null;
- }
+ public static void validatePartitionNameCharacters(List<String> partVals,
Configuration conf)
+ throws MetaException {
- for (String partVal : partVals) {
- if (!partitionValidationPattern.matcher(partVal).matches()) {
- return partVal;
- }
+ Pattern partitionValidationPattern = getPartitionValidationRegex(conf);
+ String invalidPartitionVal =
+ getPartitionValWithInvalidCharacter(partVals,
partitionValidationPattern);
+ if (invalidPartitionVal != null) {
+ throw new MetaException(
+ ("Partition value '%s' contains a character not matched by whitelist
pattern '%s'. Configure with %s for "
+ + "dynamic partitioning otherwise use metaconf:%s")
+ .formatted(
+ invalidPartitionVal,
+ partitionValidationPattern.toString(),
+
MetastoreConf.ConfVars.PARTITION_NAME_WHITELIST_PATTERN.getHiveName(),
+
MetastoreConf.ConfVars.PARTITION_NAME_WHITELIST_PATTERN.getVarname()));
}
+ }
- return null;
+ public static String getPartitionValWithInvalidCharacter(
Review Comment:
@deniskuzZ, I have a question, can you please help me clear a doubt for me?
1. FileSinkOperator#process() calling
MetaStoreServerUtils.validatePartitionNameCharacters (Line 1120) is also
coupling right? Yes/No 😅
2. 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
