HIVE-28723: Iceberg: Support metadata files clean-up

[difin]

What changes were proposed in this pull request?

Enabled Iceberg post-commit cleanup for metadata.json files.

When an Iceberg table has a table property write.metadata.delete-after-commit.enabled with the value of true,
the write.metadata.previous-versions-max table property (or the default value 100) controls how many past versions of metadata.json files should be kept.

This PR sets the following default values in Hive:
write.metadata.delete-after-commit.enabled=true
write.metadata.previous-versions-max=100

Why are the changes needed?

To support automatic maintenance for the metadata.json files, to prevent their indefinite growth.

Does this PR introduce any user-facing change?

No

How was this patch tested?

new hive-iceberg unit test; existing pre-commit tests.

[deniskuzZ]

i think it's complicated and not very friendly, can we drop the catalog-default ?

[difin]

If we drop catalog-default. it will cause ambiguity in CatalogUtils.getCatalogProperties because:

• Default catalog prefix is iceberg.catalog-default.
• Named catalog prefix is iceberg.catalog.<NAME>.

  public static Map<String, String> getCatalogProperties(Configuration conf, String catalogName) {
    Map<String, String> catalogProperties = Maps.newHashMap();
    String keyPrefix = CATALOG_CONFIG_PREFIX + catalogName;
    conf.forEach(config -> {
      if (config.getKey().startsWith(CatalogUtils.CATALOG_DEFAULT_CONFIG_PREFIX)) {
        catalogProperties.putIfAbsent(
            config.getKey().substring(CatalogUtils.CATALOG_DEFAULT_CONFIG_PREFIX.length()),
            config.getValue());
      } else if (config.getKey().startsWith(keyPrefix)) {
        catalogProperties.put(
            config.getKey().substring(keyPrefix.length() + 1),
            config.getValue());
      }
    });

[deniskuzZ]

why

public static Map<String, String> getCatalogProperties(Configuration conf, String catalogName) {
    Map<String, String> catalogProperties = Maps.newHashMap();
    String namedPrefix = CATALOG_CONFIG_PREFIX + catalogName + ".";

    conf.forEach(config -> {
        String key = config.getKey();

        if (key.startsWith("iceberg.")) {
            if (key.startsWith(namedPrefix)) {
                // Named catalog overrides default
                catalogProperties.put(
                        key.substring(namedPrefix.length()),
                        config.getValue());
            } else {
                // Default config for all catalogs
                catalogProperties.putIfAbsent(
                        key.substring("iceberg.".length()),
                        config.getValue());
            }
        }
    });

    return catalogProperties;
}

[deniskuzZ]

why do we need to remove anything?

[difin]

If these properties are always present, we need to update a lot of unit tests and q-tests whose output will be different. And if it is false, the effect is identical to the properties being unset on a table. That's why I thought to remove these properties when false. Do you think it's better to keep these settings when false and update tests?

[deniskuzZ]

yes, I think it's better to update tests than add tricks in code

[deniskuzZ]

👍 :)

[sonarqubecloud]

Quality Gate passed

Issues
0 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