Support setting extra properties in Iceberg

trinodb · Nov 5, 2024 · 3b4a388 · 3b4a388
1 parent acf98f8
commit 3b4a388
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 2 deletions.
diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
@@ -265,6 +265,7 @@
 import static io.trino.plugin.iceberg.IcebergTableName.isIcebergTableName;
 import static io.trino.plugin.iceberg.IcebergTableName.isMaterializedViewStorage;
 import static io.trino.plugin.iceberg.IcebergTableName.tableNameFrom;
+import static io.trino.plugin.iceberg.IcebergTableProperties.EXTRA_PROPERTIES_PROPERTY;
 import static io.trino.plugin.iceberg.IcebergTableProperties.FILE_FORMAT_PROPERTY;
 import static io.trino.plugin.iceberg.IcebergTableProperties.FORMAT_VERSION_PROPERTY;
 import static io.trino.plugin.iceberg.IcebergTableProperties.PARTITIONING_PROPERTY;
@@ -290,6 +291,7 @@
 import static io.trino.plugin.iceberg.IcebergUtil.getTopLevelColumns;
 import static io.trino.plugin.iceberg.IcebergUtil.newCreateTableTransaction;
 import static io.trino.plugin.iceberg.IcebergUtil.schemaFromMetadata;
+import static io.trino.plugin.iceberg.IcebergUtil.verifyExtraProperties;
 import static io.trino.plugin.iceberg.PartitionFields.parsePartitionFields;
 import static io.trino.plugin.iceberg.PartitionFields.toPartitionFields;
 import static io.trino.plugin.iceberg.SortFieldUtils.parseSortFields;
@@ -369,6 +371,7 @@ public class IcebergMetadata
     private static final String RETENTION_THRESHOLD = "retention_threshold";
     private static final String UNKNOWN_SNAPSHOT_TOKEN = "UNKNOWN";
     public static final Set<String> UPDATABLE_TABLE_PROPERTIES = ImmutableSet.<String>builder()
+            .add(EXTRA_PROPERTIES_PROPERTY)
             .add(FILE_FORMAT_PROPERTY)
             .add(FORMAT_VERSION_PROPERTY)
             .add(PARTITIONING_PROPERTY)
@@ -2127,6 +2130,14 @@ public void setTableProperties(ConnectorSession session, ConnectorTableHandle ta
         beginTransaction(icebergTable);
         UpdateProperties updateProperties = transaction.updateProperties();
 
+        if (properties.containsKey(EXTRA_PROPERTIES_PROPERTY)) {
+            //noinspection unchecked
+            Map<String, String> extraProperties = (Map<String, String>) properties.get(EXTRA_PROPERTIES_PROPERTY)
+                    .orElseThrow(() -> new IllegalArgumentException("The extra_properties property cannot be empty"));
+            verifyExtraProperties(properties.keySet(), extraProperties, allowedExtraProperties);
+            extraProperties.forEach(updateProperties::set);
+        }
+
         if (properties.containsKey(FILE_FORMAT_PROPERTY)) {
             IcebergFileFormat fileFormat = (IcebergFileFormat) properties.get(FILE_FORMAT_PROPERTY)
                     .orElseThrow(() -> new IllegalArgumentException("The format property cannot be empty"));

diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
@@ -7475,8 +7475,6 @@ public void testAlterTableWithUnsupportedProperties()
                 "The following properties cannot be updated: location, orc_bloom_filter_fpp");
         assertQueryFails("ALTER TABLE " + tableName + " SET PROPERTIES format = 'ORC', orc_bloom_filter_columns = ARRAY['a']",
                 "The following properties cannot be updated: orc_bloom_filter_columns");
-        assertQueryFails("ALTER TABLE " + tableName + " SET PROPERTIES extra_properties = MAP(ARRAY['extra.property.one'], ARRAY['foo'])",
-                "The following properties cannot be updated: extra_properties");
 
         assertUpdate("DROP TABLE " + tableName);
     }
@@ -8439,6 +8437,11 @@ public void testExtraProperties()
                 .skippingTypesCheck()
                 .matches("VALUES ('extra.property.one', 'one'), ('extra.property.two', 'two')");
 
+        assertUpdate("ALTER TABLE " + tableName + " SET PROPERTIES extra_properties = MAP(ARRAY['extra.property.one'], ARRAY['updated'])");
+        assertThat(query("SELECT key, value FROM \"" + tableName + "$properties\" WHERE key IN ('extra.property.one', 'extra.property.two')"))
+                .skippingTypesCheck()
+                .matches("VALUES ('extra.property.one', 'updated'), ('extra.property.two', 'two')");
+
         assertUpdate("DROP TABLE " + tableName);
     }
 
@@ -8517,6 +8520,22 @@ public void testIllegalExtraPropertyKey()
                 "\\QIllegal keys in extra_properties: [not_allowed_property]");
     }
 
+    @Test
+    public void testSetIllegalExtraPropertyKey()
+    {
+        try (TestTable table = new TestTable(getQueryRunner()::execute, "test_set_illegal_table_properties", "(x int)")) {
+            assertQueryFails(
+                    "ALTER TABLE " + table.getName() + " SET PROPERTIES extra_properties = MAP(ARRAY['sorted_by'], ARRAY['id'])",
+                    "\\QIllegal keys in extra_properties: [sorted_by]");
+            assertQueryFails(
+                    "ALTER TABLE " + table.getName() + " SET PROPERTIES extra_properties = MAP(ARRAY['comment'], ARRAY['some comment'])",
+                    "\\QIllegal keys in extra_properties: [comment]");
+            assertQueryFails(
+                    "ALTER TABLE " + table.getName() + " SET PROPERTIES extra_properties = MAP(ARRAY['not_allowed_property'], ARRAY['foo'])",
+                    "\\QIllegal keys in extra_properties: [not_allowed_property]");
+        }
+    }
+
     @Override
     protected Optional<SetColumnTypeSetup> filterSetColumnTypesDataProvider(SetColumnTypeSetup setup)
     {