diff --git a/docs/src/main/sphinx/connector/iceberg.md b/docs/src/main/sphinx/connector/iceberg.md index c6b30872dbc6..13043227ae7c 100644 --- a/docs/src/main/sphinx/connector/iceberg.md +++ b/docs/src/main/sphinx/connector/iceberg.md @@ -814,6 +814,11 @@ connector using a {doc}`WITH ` clause. - Comma-separated list of columns to use for Parquet bloom filter. It improves the performance of queries using Equality and IN predicates when reading Parquet files. Requires Parquet format. Defaults to `[]`. +* - ``extra_properties`` + - Additional properties added to an Iceberg table. The properties are not + used by Trino, and are available in the ``$properties`` metadata table. + The properties are not included in the output of ``SHOW CREATE TABLE`` + statements. ::: The table definition below specifies to use Parquet files, partitioning by columns 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 d1ed38fd9f9c..0aeed80d6e02 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 @@ -263,8 +263,10 @@ 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; 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.ILLEGAL_EXTRA_PROPERTIES; import static io.trino.plugin.iceberg.IcebergTableProperties.PARTITIONING_PROPERTY; import static io.trino.plugin.iceberg.IcebergTableProperties.SORTED_BY_PROPERTY; import static io.trino.plugin.iceberg.IcebergTableProperties.getPartitioning; @@ -364,7 +366,7 @@ public class IcebergMetadata private static final int CLEANING_UP_PROCEDURES_MAX_SUPPORTED_TABLE_VERSION = 2; private static final String RETENTION_THRESHOLD = "retention_threshold"; private static final String UNKNOWN_SNAPSHOT_TOKEN = "UNKNOWN"; - public static final Set UPDATABLE_TABLE_PROPERTIES = ImmutableSet.of(FILE_FORMAT_PROPERTY, FORMAT_VERSION_PROPERTY, PARTITIONING_PROPERTY, SORTED_BY_PROPERTY); + public static final Set UPDATABLE_TABLE_PROPERTIES = ImmutableSet.of(FILE_FORMAT_PROPERTY, FORMAT_VERSION_PROPERTY, PARTITIONING_PROPERTY, SORTED_BY_PROPERTY, EXTRA_PROPERTIES); public static final String NUMBER_OF_DISTINCT_VALUES_NAME = "NUMBER_OF_DISTINCT_VALUES"; private static final FunctionName NUMBER_OF_DISTINCT_VALUES_FUNCTION = new FunctionName(IcebergThetaSketchForStats.NAME); @@ -2125,6 +2127,18 @@ public void setTableProperties(ConnectorSession session, ConnectorTableHandle ta } } + if (properties.containsKey(EXTRA_PROPERTIES)) { + Map extraProperties = (Map) properties.get(EXTRA_PROPERTIES) + .orElseThrow(() -> new IllegalArgumentException("extra_properties property cannot be empty")); + + Set illegalExtraProperties = Sets.intersection(ILLEGAL_EXTRA_PROPERTIES, extraProperties.keySet()); + if (!illegalExtraProperties.isEmpty()) { + throw new TrinoException( + INVALID_TABLE_PROPERTY, + "Illegal keys in extra_properties: " + illegalExtraProperties); + } + extraProperties.forEach(updateProperties::set); + } commitTransaction(transaction, "set table properties"); } diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableProperties.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableProperties.java index 5405f88a4656..a24c3c551a80 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableProperties.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableProperties.java @@ -14,15 +14,19 @@ package io.trino.plugin.iceberg; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; import io.trino.plugin.hive.orc.OrcWriterConfig; import io.trino.spi.TrinoException; import io.trino.spi.session.PropertyMetadata; import io.trino.spi.type.ArrayType; +import io.trino.spi.type.MapType; +import io.trino.spi.type.TypeManager; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import static com.google.common.collect.ImmutableList.toImmutableList; import static io.trino.plugin.iceberg.IcebergConfig.FORMAT_VERSION_SUPPORT_MAX; @@ -46,13 +50,23 @@ public class IcebergTableProperties public static final String ORC_BLOOM_FILTER_COLUMNS_PROPERTY = "orc_bloom_filter_columns"; public static final String ORC_BLOOM_FILTER_FPP_PROPERTY = "orc_bloom_filter_fpp"; public static final String PARQUET_BLOOM_FILTER_COLUMNS_PROPERTY = "parquet_bloom_filter_columns"; + public static final String EXTRA_PROPERTIES = "extra_properties"; + public static final Set ILLEGAL_EXTRA_PROPERTIES = ImmutableSet.of( + FILE_FORMAT_PROPERTY, + PARTITIONING_PROPERTY, + SORTED_BY_PROPERTY, + LOCATION_PROPERTY, + FORMAT_VERSION_PROPERTY, + ORC_BLOOM_FILTER_COLUMNS, + ORC_BLOOM_FILTER_FPP); private final List> tableProperties; @Inject public IcebergTableProperties( IcebergConfig icebergConfig, - OrcWriterConfig orcWriterConfig) + OrcWriterConfig orcWriterConfig, + TypeManager typeManager) { tableProperties = ImmutableList.>builder() .add(enumProperty( @@ -120,6 +134,24 @@ public IcebergTableProperties( .map(name -> name.toLowerCase(ENGLISH)) .collect(toImmutableList()), value -> value)) + .add(new PropertyMetadata<>( + EXTRA_PROPERTIES, + "Extra table properties", + new MapType(VARCHAR, VARCHAR, typeManager.getTypeOperators()), + Map.class, + null, + true, // These properties are not listed in SHOW CREATE TABLE + value -> { + Map extraProperties = (Map) value; + if (extraProperties.containsValue(null)) { + throw new TrinoException(INVALID_TABLE_PROPERTY, format("Extra table property value cannot be null '%s'", extraProperties)); + } + if (extraProperties.containsKey(null)) { + throw new TrinoException(INVALID_TABLE_PROPERTY, format("Extra table property key cannot be null '%s'", extraProperties)); + } + return extraProperties; + }, + value -> value)) .build(); } @@ -188,4 +220,9 @@ public static List getParquetBloomFilterColumns(Map tabl List parquetBloomFilterColumns = (List) tableProperties.get(PARQUET_BLOOM_FILTER_COLUMNS_PROPERTY); return parquetBloomFilterColumns == null ? ImmutableList.of() : ImmutableList.copyOf(parquetBloomFilterColumns); } + + public static Optional> getExtraProperties(Map tableProperties) + { + return Optional.ofNullable((Map) tableProperties.get(EXTRA_PROPERTIES)); + } } diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java index 84820e882ce6..fd84bb4525dd 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java @@ -118,12 +118,16 @@ import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_INVALID_PARTITION_VALUE; 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.ILLEGAL_EXTRA_PROPERTIES; import static io.trino.plugin.iceberg.IcebergTableProperties.LOCATION_PROPERTY; import static io.trino.plugin.iceberg.IcebergTableProperties.ORC_BLOOM_FILTER_COLUMNS_PROPERTY; import static io.trino.plugin.iceberg.IcebergTableProperties.ORC_BLOOM_FILTER_FPP_PROPERTY; import static io.trino.plugin.iceberg.IcebergTableProperties.PARQUET_BLOOM_FILTER_COLUMNS_PROPERTY; import static io.trino.plugin.iceberg.IcebergTableProperties.PARTITIONING_PROPERTY; import static io.trino.plugin.iceberg.IcebergTableProperties.SORTED_BY_PROPERTY; +import static io.trino.plugin.iceberg.IcebergTableProperties.getExtraProperties; +import static io.trino.plugin.iceberg.IcebergTableProperties.getOrcBloomFilterColumns; +import static io.trino.plugin.iceberg.IcebergTableProperties.getOrcBloomFilterFpp; import static io.trino.plugin.iceberg.IcebergTableProperties.getPartitioning; import static io.trino.plugin.iceberg.IcebergTableProperties.getSortOrder; import static io.trino.plugin.iceberg.PartitionFields.parsePartitionFields; @@ -832,7 +836,25 @@ public static Map createTableProperties(ConnectorTableMetadata t if (tableMetadata.getComment().isPresent()) { propertiesBuilder.put(TABLE_COMMENT, tableMetadata.getComment().get()); } - return propertiesBuilder.buildOrThrow(); + + Map baseProperties = propertiesBuilder.buildOrThrow(); + + // Add properties set via "extra_properties" table property. + Map extraProperties = getExtraProperties(tableMetadata.getProperties()) + .orElseGet(ImmutableMap::of); + Set illegalExtraProperties = Sets.intersection(ILLEGAL_EXTRA_PROPERTIES, extraProperties.keySet()); + if (!illegalExtraProperties.isEmpty()) { + throw new TrinoException( + INVALID_TABLE_PROPERTY, + "Illegal keys in extra_properties: " + illegalExtraProperties); + } + + Map properties = ImmutableMap.builder() + .putAll(baseProperties) + .putAll(extraProperties) + .buildOrThrow(); + + return catalog.newCreateTableTransaction(session, schemaTableName, schema, partitionSpec, sortOrder, targetPath, properties); } /** 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 23261c9a698d..bb017505ec7c 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 @@ -48,6 +48,7 @@ import io.trino.testing.DistributedQueryRunner; import io.trino.testing.MaterializedResult; import io.trino.testing.MaterializedRow; +import io.trino.testing.QueryFailedException; import io.trino.testing.QueryRunner; import io.trino.testing.QueryRunner.MaterializedResultWithPlan; import io.trino.testing.TestingConnectorBehavior; @@ -62,6 +63,7 @@ import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.io.FileIO; import org.apache.iceberg.util.JsonUtil; +import org.assertj.core.api.Condition; import org.intellij.lang.annotations.Language; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -8577,4 +8579,105 @@ private void assertQueryIdStored(String tableName, QueryId queryId) assertThat(getFieldFromLatestSnapshotSummary(tableName, TRINO_QUERY_ID_NAME)) .isEqualTo(queryId.toString()); } + + @Test + public void testExtraProperties() + { + String tableName = format("%s.%s.create_table_with_multiple_extra_properties_%s", getSession().getCatalog().get(), getSession().getSchema().get(), randomNameSuffix()); + assertUpdate("CREATE TABLE %s (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property.one', 'extra.property.two'], ARRAY['one', 'two']))".formatted(tableName)); + + assertQuery( + "SELECT \"extra.property.one\", \"extra.property.two\" FROM \"%s$properties\"".formatted(tableName), + "SELECT 'one', 'two'"); + + // Assert that SHOW CREATE TABLE does not contain extra_properties + assertThat((String) computeActual("SHOW CREATE TABLE %s".formatted(tableName)).getOnlyValue()) + .satisfies(new Condition<>( + queryResult -> queryResult.contains("extra_properties"), "noExtraProperties" + )); + + assertUpdate("DROP TABLE %s".formatted(tableName)); + } + + @Test + public void testExtraPropertiesWithCtas() + { + String tableName = format("%s.%s.create_table_ctas_with_multiple_extra_properties_%s", getSession().getCatalog().get(), getSession().getSchema().get(), randomNameSuffix()); + assertUpdate("CREATE TABLE %s (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property.one', 'extra.property.two'], ARRAY['one', 'two']))".formatted(tableName)); + + assertQuery( + "SELECT \"extra.property.one\", \"extra.property.two\" FROM \"%s$properties\"".formatted(tableName), + "SELECT 'one', 'two'"); + + // Assert that SHOW CREATE TABLE does not contain extra_properties + assertThat((String) computeActual("SHOW CREATE TABLE %s".formatted(tableName)).getOnlyValue()) + .satisfies(new Condition<>( + queryResult -> queryResult.contains("extra_properties"), "noExtraProperties" + )); + + assertUpdate("DROP TABLE %s".formatted(tableName)); + } + + @Test + public void testShowCreateWithExtraProperties() + { + String tableName = format("%s.%s.show_create_table_with_extra_properties_%s", getSession().getCatalog().get(), getSession().getSchema().get(), randomNameSuffix()); + assertUpdate("CREATE TABLE %s (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property.one', 'extra.property.two'], ARRAY['one', 'two']))".formatted(tableName)); + + // Assert that SHOW CREATE TABLE does not contain extra_properties + assertThat((String) computeActual("SHOW CREATE TABLE " + tableName).getOnlyValue()) + .satisfies(new Condition<>( + queryResult -> queryResult.contains("extra_properties"), "noExtraProperties" + )); + + assertUpdate("DROP TABLE %s".formatted(tableName)); + } + + @Test + public void testDuplicateExtraProperties() + { + assertQueryFails( + "CREATE TABLE create_table_with_duplicate_extra_properties (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property', 'extra.property'], ARRAY['true', 'false']))", + "Invalid value for catalog 'iceberg' table property 'extra_properties': Cannot convert.*"); + assertQueryFails( + "CREATE TABLE create_table_select_as_with_duplicate_extra_properties (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property', 'extra.property'], ARRAY['true', 'false']))", + "Invalid value for catalog 'iceberg' table property 'extra_properties': Cannot convert.*"); + } + + @Test + public void testOverwriteExistingPropertyWithExtraProperties() + { + assertThatThrownBy(() -> assertUpdate("CREATE TABLE create_table_with_overwrite_extra_properties (c1 integer) WITH (extra_properties = MAP(ARRAY['write.format.default'], ARRAY['foobar']))")) + .isInstanceOf(QueryFailedException.class) + .hasMessage("Illegal keys in extra_properties: [write.format.default]"); + + assertThatThrownBy(() -> assertUpdate("CREATE TABLE create_table_as_select_with_extra_properties WITH (extra_properties = MAP(ARRAY['format-version'], ARRAY['10'])) AS SELECT 1 as c1")) + .isInstanceOf(QueryFailedException.class) + .hasMessage("Illegal keys in extra_properties: [format-version]"); + } + + @Test + public void testNullExtraProperty() + { + assertQueryFails( + "CREATE TABLE create_table_with_duplicate_extra_properties (c1 integer) WITH (extra_properties = MAP(ARRAY['null.property'], ARRAY[null]))", + ".*Extra table property value cannot be null '\\{null.property=null}'.*"); + assertQueryFails( + "CREATE TABLE create_table_as_select_with_extra_properties WITH (extra_properties = MAP(ARRAY['null.property'], ARRAY[null])) AS SELECT 1 as c1", + ".*Extra table property value cannot be null '\\{null.property=null}'.*"); + } + + @Test + public void testCollidingMixedCaseProperty() + { + String tableName = "create_table_with_mixed_case_extra_properties" + randomNameSuffix(); + + assertUpdate("CREATE TABLE %s (c1 integer) WITH (extra_properties = MAP(ARRAY['one', 'ONE'], ARRAY['one', 'ONE']))".formatted(tableName)); + // TODO: (https://github.com/trinodb/trino/issues/17) This should run successfully + assertThatThrownBy(() -> query("SELECT * FROM \"%s$properties\"".formatted(tableName))) + .isInstanceOf(QueryFailedException.class) + .hasMessageContaining("Multiple entries with same key: one=one and one=one"); + + assertUpdate("DROP TABLE %s".formatted(tableName)); + } }