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 index b75aea3b3efcf..82e7195acaa85 100644 --- 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 UPDATABLE_TABLE_PROPERTIES = ImmutableSet.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 extraProperties = (Map) 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 index b4faa0268f232..266d35e961363 100644 --- 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 filterSetColumnTypesDataProvider(SetColumnTypeSetup setup) {