HIVE-29089: Fix MSCK Repair table adding invalid partitions for non string partitions columns

[vikramahuja1001]

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?

[deniskuzZ]

maybe ColumnType.getTypeName(type)? this would handle the type.startsWith("decimal")

[deniskuzZ]

why tbl_y:month=12/day=__HIVE_DEFAULT_PARTITION__ is gone?

[deniskuzZ]

same here

[deniskuzZ]

why check for null, if we skip the iteration when partitionName == null?

[deniskuzZ]

i don't understand this change:

if the partitionValue is not a number but type is String, we'll attempt to parse the number in partitionValue - doesn't make any sense

DEFAULTPARTITIONNAME only applies to String type

[deniskuzZ]

I see it like

public static String getNormalisedPartitionValue(String partitionValue, String type, Configuration conf) {
  // 1. Handle simple exit cases first.
  if (type == null) {
    return partitionValue;
  }
  if (Objects.equals(partitionValue, MetastoreConf.getVar(conf,
        MetastoreConf.ConfVars.DEFAULTPARTITIONNAME))) {
    // This is the special partition name for NULL values. It should never be parsed.
    return partitionValue;
  }
  // 2. If the type is not numeric, no normalization is needed.
  String colType = ColumnType.getTypeName(type);
  if (!ColumnType.NumericTypes.contains(colType)) {
    return partitionValue;
  }
  // 3. At this point, we have a numeric type that needs normalization.
  LOG.debug("Normalizing partition value '{}' for type '{}'.", partitionValue, type);
  try {
    switch (colType) {
      case ColumnType.TINYINT_TYPE_NAME:
      case ColumnType.SMALLINT_TYPE_NAME:
      case ColumnType.INT_TYPE_NAME:
        return Integer.toString(Integer.parseInt(partitionValue));
      case ColumnType.BIGINT_TYPE_NAME:
        return Long.toString(Long.parseLong(partitionValue));
      case ColumnType.FLOAT_TYPE_NAME:
        return Float.toString(Float.parseFloat(partitionValue));
      case ColumnType.DOUBLE_TYPE_NAME:
        return Double.toString(Double.parseDouble(partitionValue));
      case ColumnType.DECIMAL_TYPE_NAME:
        return new BigDecimal(partitionValue).stripTrailingZeros().toPlainString();
    }
  } catch (NumberFormatException e) {
    // 4. Handle cases where the value cannot be parsed as the expected number type.
    String validationMode = MetastoreConf.getVar(conf, MetastoreConf.ConfVars.MSCK_PATH_VALIDATION);
    if ("throw".equals(validationMode)) {
      LOG.error("Invalid partition value: Cannot parse '{}' as type '{}'. Failing MSCK. "
          + "Set hive.msck.path.validation=skip to ignore invalid partitions.", partitionValue, type);
      throw e;
    } else if ("skip".equals(validationMode)) {
      LOG.warn("Skipping invalid partition value '{}' for type '{}' due to parsing error.", partitionValue, type);
      // Signals the caller to skip this partition.
      return null;
    }
  }
  return partitionValue;
}

[deniskuzZ]

@vikramahuja1001, please check the comments

[sonarqubecloud]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud