diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSchemaProperties.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSchemaProperties.java index bdc43ab1b9e3..714403fa5299 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSchemaProperties.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergSchemaProperties.java @@ -14,10 +14,12 @@ 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.spi.session.PropertyMetadata; import java.util.List; +import java.util.Set; import static io.trino.spi.session.PropertyMetadata.stringProperty; @@ -25,6 +27,10 @@ public final class IcebergSchemaProperties { public static final String LOCATION_PROPERTY = "location"; + public static final Set SUPPORTED_SCHEMA_PROPERTIES = ImmutableSet.builder() + .add(LOCATION_PROPERTY) + .build(); + public final List> schemaProperties; @Inject diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java index a2760ee91897..22ef11834d68 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java @@ -292,9 +292,6 @@ public Map loadNamespaceMetadata(ConnectorSession session, Strin if (database.locationUri() != null) { metadata.put(LOCATION_PROPERTY, database.locationUri()); } - if (database.parameters() != null) { - metadata.putAll(database.parameters()); - } return metadata.buildOrThrow(); } catch (EntityNotFoundException e) { diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java index 1c18177d5b4a..a18b0ac436eb 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java @@ -59,9 +59,11 @@ import static com.google.common.base.Throwables.throwIfUnchecked; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static io.trino.cache.CacheUtils.uncheckedCacheGet; import static io.trino.filesystem.Locations.appendPath; import static io.trino.plugin.iceberg.IcebergSchemaProperties.LOCATION_PROPERTY; +import static io.trino.plugin.iceberg.IcebergSchemaProperties.SUPPORTED_SCHEMA_PROPERTIES; import static io.trino.plugin.iceberg.IcebergUtil.getIcebergTableWithMetadata; import static io.trino.plugin.iceberg.IcebergUtil.quotedTableName; import static io.trino.plugin.iceberg.catalog.nessie.IcebergNessieUtil.toIdentifier; @@ -125,7 +127,9 @@ public void dropNamespace(ConnectorSession session, String namespace) public Map loadNamespaceMetadata(ConnectorSession session, String namespace) { try { - return ImmutableMap.copyOf(nessieClient.loadNamespaceMetadata(Namespace.of(namespace))); + return nessieClient.loadNamespaceMetadata(Namespace.of(namespace)).entrySet().stream() + .filter(metadata -> SUPPORTED_SCHEMA_PROPERTIES.contains(metadata.getKey())) + .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); } catch (NoSuchNamespaceException e) { throw new SchemaNotFoundException(namespace); diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoRestCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoRestCatalog.java index 90467c027374..751dd20fe41d 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoRestCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoRestCatalog.java @@ -26,7 +26,6 @@ import io.trino.cache.EvictableCacheBuilder; import io.trino.metastore.TableInfo; import io.trino.plugin.iceberg.ColumnIdentity; -import io.trino.plugin.iceberg.IcebergSchemaProperties; import io.trino.plugin.iceberg.IcebergUtil; import io.trino.plugin.iceberg.catalog.TrinoCatalog; import io.trino.plugin.iceberg.catalog.rest.IcebergRestCatalogConfig.SessionType; @@ -85,11 +84,14 @@ import java.util.stream.Stream; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static io.trino.cache.CacheUtils.uncheckedCacheGet; import static io.trino.filesystem.Locations.appendPath; import static io.trino.metastore.Table.TABLE_COMMENT; import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_CATALOG_ERROR; import static io.trino.plugin.iceberg.IcebergErrorCode.ICEBERG_UNSUPPORTED_VIEW_DIALECT; +import static io.trino.plugin.iceberg.IcebergSchemaProperties.LOCATION_PROPERTY; +import static io.trino.plugin.iceberg.IcebergSchemaProperties.SUPPORTED_SCHEMA_PROPERTIES; import static io.trino.plugin.iceberg.IcebergUtil.quotedTableName; import static io.trino.plugin.iceberg.catalog.AbstractTrinoCatalog.ICEBERG_VIEW_RUN_AS_OWNER; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; @@ -221,7 +223,9 @@ public Map loadNamespaceMetadata(ConnectorSession session, Strin { try { // Return immutable metadata as direct modifications will not be reflected on the namespace - return ImmutableMap.copyOf(restSessionCatalog.loadNamespaceMetadata(convert(session), toRemoteNamespace(session, toNamespace(namespace)))); + return restSessionCatalog.loadNamespaceMetadata(convert(session), toRemoteNamespace(session, toNamespace(namespace))).entrySet().stream() + .filter(property -> SUPPORTED_SCHEMA_PROPERTIES.contains(property.getKey())) + .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); } catch (NoSuchNamespaceException e) { throw new SchemaNotFoundException(namespace); @@ -576,7 +580,7 @@ public String defaultTableLocation(ConnectorSession session, SchemaTableName sch String tableName = createLocationForTable(schemaTableName.getTableName()); Map properties = loadNamespaceMetadata(session, schemaTableName.getSchemaName()); - String databaseLocation = (String) properties.get(IcebergSchemaProperties.LOCATION_PROPERTY); + String databaseLocation = (String) properties.get(LOCATION_PROPERTY); if (databaseLocation == null) { // Iceberg REST catalog doesn't require location property. // S3 Tables doesn't return the property. diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java index ebac31ac9970..e7eaea463184 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java @@ -167,6 +167,40 @@ public void testNonLowercaseNamespace() } } + @Test + public void testSchemaWithInvalidProperties() + throws Exception + { + String namespace = "test_schema_invalid_properties" + randomNameSuffix(); + + TrinoCatalog catalog = createTrinoCatalog(false); + createNamespaceWithProperties(catalog, namespace, ImmutableMap.of("invalid_property", "test-value")); + try { + ConnectorMetadata icebergMetadata = new IcebergMetadata( + PLANNER_CONTEXT.getTypeManager(), + jsonCodec(CommitTaskData.class), + catalog, + (_, _) -> { + throw new UnsupportedOperationException(); + }, + TABLE_STATISTICS_READER, + new TableStatisticsWriter(new NodeVersion("test-version")), + Optional.empty(), + false, + _ -> false, + newDirectExecutorService(), + directExecutor(), + newDirectExecutorService(), + newDirectExecutorService()); + + assertThat(icebergMetadata.getSchemaProperties(SESSION, namespace)) + .doesNotContainKey("invalid_property"); + } + finally { + catalog.dropNamespace(SESSION, namespace); + } + } + @Test public void testCreateTable() throws Exception @@ -536,6 +570,8 @@ public void testListTables() } } + protected abstract void createNamespaceWithProperties(TrinoCatalog catalog, String namespace, Map properties); + protected void createMaterializedView( ConnectorSession session, TrinoCatalog catalog, diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/file/TestTrinoHiveCatalogWithFileMetastore.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/file/TestTrinoHiveCatalogWithFileMetastore.java index 344330154420..2670ec2e1bce 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/file/TestTrinoHiveCatalogWithFileMetastore.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/file/TestTrinoHiveCatalogWithFileMetastore.java @@ -19,6 +19,7 @@ import io.trino.filesystem.Location; import io.trino.filesystem.TrinoFileSystemFactory; import io.trino.filesystem.local.LocalFileSystemFactory; +import io.trino.metastore.Database; import io.trino.metastore.HiveMetastore; import io.trino.metastore.cache.CachingHiveMetastore; import io.trino.plugin.hive.TrinoViewHiveMetastore; @@ -43,6 +44,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Map; import java.util.Optional; import static com.google.common.io.MoreFiles.deleteRecursively; @@ -89,6 +91,17 @@ public void tearDown() deleteRecursively(tempDir, ALLOW_INSECURE); } + @Override + protected void createNamespaceWithProperties(TrinoCatalog catalog, String namespace, Map properties) + { + metastore.createDatabase(Database.builder() + .setDatabaseName(namespace) + .setOwnerName(Optional.of("test")) + .setOwnerType(Optional.of(PrincipalType.USER)) + .setParameters(properties) + .build()); + } + @Override protected TrinoCatalog createTrinoCatalog(boolean useUniqueTableLocations) { diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestTrinoGlueCatalog.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestTrinoGlueCatalog.java index 79400318e0be..3ddacdd76311 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestTrinoGlueCatalog.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestTrinoGlueCatalog.java @@ -74,6 +74,15 @@ protected TrinoCatalog createTrinoCatalog(boolean useUniqueTableLocations) return createGlueTrinoCatalog(useUniqueTableLocations, false); } + @Override + protected void createNamespaceWithProperties(TrinoCatalog catalog, String namespace, Map properties) + { + try (GlueClient glueClient = GlueClient.create()) { + glueClient.createDatabase(database -> database + .databaseInput(input -> input.name(namespace).parameters(properties))); + } + } + private TrinoCatalog createGlueTrinoCatalog(boolean useUniqueTableLocations, boolean useSystemSecurity) { GlueClient glueClient = GlueClient.create(); diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestTrinoHiveCatalogWithHiveMetastore.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestTrinoHiveCatalogWithHiveMetastore.java index db7166598d3a..0cd2880fb8f8 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestTrinoHiveCatalogWithHiveMetastore.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/hms/TestTrinoHiveCatalogWithHiveMetastore.java @@ -24,6 +24,7 @@ import io.trino.filesystem.s3.S3FileSystemConfig; import io.trino.filesystem.s3.S3FileSystemFactory; import io.trino.filesystem.s3.S3FileSystemStats; +import io.trino.metastore.Database; import io.trino.metastore.Table; import io.trino.metastore.TableInfo; import io.trino.metastore.cache.CachingHiveMetastore; @@ -67,6 +68,7 @@ import static io.trino.metastore.cache.CachingHiveMetastore.createPerTransactionCache; import static io.trino.plugin.hive.TestingThriftHiveMetastoreBuilder.testingThriftHiveMetastoreBuilder; import static io.trino.plugin.hive.containers.HiveHadoop.HIVE3_IMAGE; +import static io.trino.plugin.hive.metastore.thrift.ThriftMetastoreUtil.toMetastoreApiDatabase; import static io.trino.plugin.iceberg.IcebergFileFormat.PARQUET; import static io.trino.plugin.iceberg.IcebergTableProperties.FILE_FORMAT_PROPERTY; import static io.trino.plugin.iceberg.IcebergTableProperties.FORMAT_VERSION_PROPERTY; @@ -119,6 +121,20 @@ public void tearDown() closer.close(); } + @Override + protected void createNamespaceWithProperties(TrinoCatalog catalog, String namespace, Map properties) + { + ThriftMetastore thriftMetastore = testingThriftHiveMetastoreBuilder() + .metastoreClient(dataLake.getHiveMetastoreEndpoint()) + .build(closer::register); + thriftMetastore.createDatabase(toMetastoreApiDatabase(Database.builder() + .setDatabaseName(namespace) + .setOwnerName(Optional.of("test")) + .setOwnerType(Optional.of(PrincipalType.USER)) + .setParameters(properties) + .build())); + } + @Override protected TrinoCatalog createTrinoCatalog(boolean useUniqueTableLocations) { diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/nessie/TestTrinoNessieCatalog.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/nessie/TestTrinoNessieCatalog.java index d69d40db4bd1..c4549d64e9a2 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/nessie/TestTrinoNessieCatalog.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/nessie/TestTrinoNessieCatalog.java @@ -29,6 +29,7 @@ import io.trino.spi.security.PrincipalType; import io.trino.spi.security.TrinoPrincipal; import io.trino.spi.type.TestingTypeManager; +import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.nessie.NessieIcebergClient; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -84,6 +85,18 @@ public void teardownServer() } } + @Override + protected void createNamespaceWithProperties(TrinoCatalog catalog, String namespace, Map properties) + { + IcebergNessieCatalogConfig icebergNessieCatalogConfig = new IcebergNessieCatalogConfig() + .setServerUri(URI.create(nessieContainer.getRestApiUri())); + NessieApiV2 nessieApi = NessieClientBuilder.createClientBuilderFromSystemSettings() + .withUri(nessieContainer.getRestApiUri()) + .build(NessieApiV2.class); + NessieIcebergClient nessieClient = new NessieIcebergClient(nessieApi, icebergNessieCatalogConfig.getDefaultReferenceName(), null, ImmutableMap.of()); + nessieClient.createNamespace(Namespace.of(namespace), properties); + } + @Override protected TrinoCatalog createTrinoCatalog(boolean useUniqueTableLocations) { diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestTrinoRestCatalog.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestTrinoRestCatalog.java index 884a1f889c65..12094a1bae1a 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestTrinoRestCatalog.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestTrinoRestCatalog.java @@ -38,6 +38,7 @@ import java.util.Map; import java.util.Optional; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.util.concurrent.MoreExecutors.directExecutor; import static com.google.common.util.concurrent.MoreExecutors.newDirectExecutorService; import static io.airlift.json.JsonCodec.jsonCodec; @@ -62,6 +63,17 @@ protected TrinoCatalog createTrinoCatalog(boolean useUniqueTableLocations) return createTrinoRestCatalog(useUniqueTableLocations, ImmutableMap.of()); } + @Override + protected void createNamespaceWithProperties(TrinoCatalog catalog, String namespace, Map properties) + { + catalog.createNamespace( + SESSION, + namespace, + properties.entrySet().stream() + .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)), + new TrinoPrincipal(PrincipalType.USER, SESSION.getUser())); + } + private static TrinoRestCatalog createTrinoRestCatalog(boolean useUniqueTableLocations, Map properties) throws IOException { diff --git a/plugin/trino-iceberg/src/test/java/org/apache/iceberg/snowflake/TestTrinoSnowflakeCatalog.java b/plugin/trino-iceberg/src/test/java/org/apache/iceberg/snowflake/TestTrinoSnowflakeCatalog.java index 5472c9422280..0d2e47cc31c7 100644 --- a/plugin/trino-iceberg/src/test/java/org/apache/iceberg/snowflake/TestTrinoSnowflakeCatalog.java +++ b/plugin/trino-iceberg/src/test/java/org/apache/iceberg/snowflake/TestTrinoSnowflakeCatalog.java @@ -48,6 +48,7 @@ import org.apache.iceberg.expressions.Expressions; import org.apache.iceberg.jdbc.JdbcClientPool; import org.apache.iceberg.types.Types; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -148,6 +149,12 @@ private static void executeOnSnowflake(TestingSnowflakeServer server, String sql server.execute(SNOWFLAKE_TEST_SCHEMA, sql); } + @Override + protected void createNamespaceWithProperties(TrinoCatalog catalog, String namespace, Map namespaceProperties) + { + Assumptions.abort("Snowflake catalog does not support creating namespaces"); + } + @Override protected TrinoCatalog createTrinoCatalog(boolean useUniqueTableLocations) {