Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support setting extra properties in Iceberg #24031

Merged
merged 3 commits into from
Nov 5, 2024
Merged

Support setting extra properties in Iceberg #24031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aea581f
Use builder for IcebergMetadata.UPDATABLE_TABLE_PROPERTIES
ebyhr Nov 5, 2024
dcf3090
Extract helper method to verify extra properties in Iceberg
ebyhr Nov 5, 2024
1571c86
Support setting extra properties in Iceberg
ebyhr Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -368,7 +370,13 @@ 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<String> UPDATABLE_TABLE_PROPERTIES = ImmutableSet.of(FILE_FORMAT_PROPERTY, FORMAT_VERSION_PROPERTY, PARTITIONING_PROPERTY, SORTED_BY_PROPERTY);
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)
.add(SORTED_BY_PROPERTY)
.build();

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);
Expand Down Expand Up @@ -2122,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"));
Expand Down
17 changes: 11 additions & 6 deletions plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,11 +839,21 @@ public static Map<String, String> createTableProperties(ConnectorTableMetadata t
Map<String, String> baseProperties = propertiesBuilder.buildOrThrow();
Map<String, String> extraProperties = IcebergTableProperties.getExtraProperties(tableMetadata.getProperties()).orElseGet(ImmutableMap::of);

verifyExtraProperties(baseProperties.keySet(), extraProperties, allowedExtraProperties);

return ImmutableMap.<String, String>builder()
.putAll(baseProperties)
.putAll(extraProperties)
.buildOrThrow();
}

public static void verifyExtraProperties(Set<String> basePropertyKeys, Map<String, String> extraProperties, Predicate<String> allowedExtraProperties)
{
Set<String> illegalExtraProperties = ImmutableSet.<String>builder()
.addAll(Sets.intersection(
ImmutableSet.<String>builder()
.add(TABLE_COMMENT)
.addAll(baseProperties.keySet())
.addAll(basePropertyKeys)
.addAll(SUPPORTED_PROPERTIES)
.addAll(PROTECTED_ICEBERG_NATIVE_PROPERTIES)
.build(),
Expand All @@ -858,11 +868,6 @@ public static Map<String, String> createTableProperties(ConnectorTableMetadata t
INVALID_TABLE_PROPERTY,
format("Illegal keys in extra_properties: %s", illegalExtraProperties));
}

return ImmutableMap.<String, String>builder()
.putAll(baseProperties)
.putAll(extraProperties)
.buildOrThrow();
}

/**
Expand Down
23 changes: 21 additions & 2 deletions plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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)
{
Expand Down