Remove iceberg.rest-catalog.parent-namespace property

ebyhr · ebyhr · commit f76e3c344e15 · 2024-11-28T07:32:21.000+09:00
The property was added for supporting Unity catalog. They changed the behavior to respect warehouse property in unitycatalog/unitycatalog#324.
diff --git a/docs/src/main/sphinx/object-storage/metastores.md b/docs/src/main/sphinx/object-storage/metastores.md
@@ -484,9 +484,6 @@ following properties:
 * - `iceberg.rest-catalog.warehouse`
   - Warehouse identifier/location for the catalog (optional). Example:
     `s3://my_bucket/warehouse_location`
-* - `iceberg.rest-catalog.parent-namespace`
-  - The namespace to use with the REST catalog server. Example:
-    `main`
 * - `iceberg.rest-catalog.security`
   - The type of security to use (default: `NONE`).  `OAUTH2` requires either a
     `token` or `credential`. Example: `OAUTH2`
@@ -537,7 +534,6 @@ iceberg.rest-catalog.uri=https://dbc-12345678-9999.cloud.databricks.com/api/2.1/
 iceberg.security=read_only
 iceberg.rest-catalog.security=OAUTH2
 iceberg.rest-catalog.oauth2.token=***
-iceberg.rest-catalog.parent-namespace=test_namespace
 ```
 
 The REST catalog supports [view management](sql-view-management) 
diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogConfig.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogConfig.java
@@ -15,16 +15,17 @@
 
 import io.airlift.configuration.Config;
 import io.airlift.configuration.ConfigDescription;
+import io.airlift.configuration.DefunctConfig;
 import io.airlift.units.Duration;
 import io.airlift.units.MinDuration;
 import jakarta.validation.constraints.NotNull;
-import org.apache.iceberg.catalog.Namespace;
 
 import java.net.URI;
 import java.util.Optional;
 
 import static java.util.concurrent.TimeUnit.MINUTES;
 
+@DefunctConfig("iceberg.rest-catalog.parent-namespace")
 public class IcebergRestCatalogConfig
 {
     public enum Security
@@ -42,7 +43,6 @@ public enum SessionType
     private URI restUri;
     private Optional<String> prefix = Optional.empty();
     private Optional<String> warehouse = Optional.empty();
-    private Namespace parentNamespace = Namespace.of();
     private boolean nestedNamespaceEnabled;
     private Security security = Security.NONE;
     private SessionType sessionType = SessionType.NONE;
@@ -92,19 +92,6 @@ public IcebergRestCatalogConfig setWarehouse(String warehouse)
         return this;
     }
 
-    public Namespace getParentNamespace()
-    {
-        return parentNamespace;
-    }
-
-    @Config("iceberg.rest-catalog.parent-namespace")
-    @ConfigDescription("The parent namespace to use with the REST catalog server")
-    public IcebergRestCatalogConfig setParentNamespace(String parentNamespace)
-    {
-        this.parentNamespace = parentNamespace == null ? Namespace.empty() : Namespace.of(parentNamespace);
-        return this;
-    }
-
     public boolean isNestedNamespaceEnabled()
     {
         return nestedNamespaceEnabled;
diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoIcebergRestCatalogFactory.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoIcebergRestCatalogFactory.java
@@ -54,7 +54,6 @@ public class TrinoIcebergRestCatalogFactory
     private final URI serverUri;
     private final Optional<String> prefix;
     private final Optional<String> warehouse;
-    private final Namespace parentNamespace;
     private final boolean nestedNamespaceEnabled;
     private final SessionType sessionType;
     private final boolean vendedCredentialsEnabled;
@@ -85,7 +84,6 @@ public TrinoIcebergRestCatalogFactory(
         this.serverUri = restConfig.getBaseUri();
         this.prefix = restConfig.getPrefix();
         this.warehouse = restConfig.getWarehouse();
-        this.parentNamespace = restConfig.getParentNamespace();
         this.nestedNamespaceEnabled = restConfig.isNestedNamespaceEnabled();
         this.sessionType = restConfig.getSessionType();
         this.vendedCredentialsEnabled = restConfig.isVendedCredentialsEnabled();
@@ -144,7 +142,6 @@ public synchronized TrinoCatalog create(ConnectorIdentity identity)
                 catalogName,
                 sessionType,
                 credentials,
-                parentNamespace,
                 nestedNamespaceEnabled,
                 trinoVersion,
                 typeManager,
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
@@ -97,7 +97,6 @@
 import static java.util.Locale.ENGLISH;
 import static java.util.Objects.requireNonNull;
 import static java.util.UUID.randomUUID;
-import static java.util.stream.Collectors.joining;
 import static org.apache.iceberg.view.ViewProperties.COMMENT;
 
 public class TrinoRestCatalog
@@ -113,7 +112,6 @@ public class TrinoRestCatalog
     private final TypeManager typeManager;
     private final SessionType sessionType;
     private final Map<String, String> credentials;
-    private final Namespace parentNamespace;
     private final boolean nestedNamespaceEnabled;
     private final String trinoVersion;
     private final boolean useUniqueTableLocation;
@@ -130,7 +128,6 @@ public TrinoRestCatalog(
             CatalogName catalogName,
             SessionType sessionType,
             Map<String, String> credentials,
-            Namespace parentNamespace,
             boolean nestedNamespaceEnabled,
             String trinoVersion,
             TypeManager typeManager,
@@ -143,7 +140,6 @@ public TrinoRestCatalog(
         this.catalogName = requireNonNull(catalogName, "catalogName is null");
         this.sessionType = requireNonNull(sessionType, "sessionType is null");
         this.credentials = ImmutableMap.copyOf(requireNonNull(credentials, "credentials is null"));
-        this.parentNamespace = requireNonNull(parentNamespace, "parentNamespace is null");
         this.nestedNamespaceEnabled = nestedNamespaceEnabled;
         this.trinoVersion = requireNonNull(trinoVersion, "trinoVersion is null");
         this.typeManager = requireNonNull(typeManager, "typeManager is null");
@@ -169,9 +165,9 @@ public boolean namespaceExists(ConnectorSession session, String namespace)
     public List<String> listNamespaces(ConnectorSession session)
     {
         if (nestedNamespaceEnabled) {
-            return collectNamespaces(session, parentNamespace);
+            return collectNamespaces(session, Namespace.empty());
         }
-        return restSessionCatalog.listNamespaces(convert(session), parentNamespace).stream()
+        return restSessionCatalog.listNamespaces(convert(session)).stream()
                 .map(this::toSchemaName)
                 .collect(toImmutableList());
     }
@@ -732,25 +728,15 @@ private Namespace toNamespace(String schemaName)
         if (!nestedNamespaceEnabled && schemaName.contains(NAMESPACE_SEPARATOR)) {
             throw new TrinoException(NOT_SUPPORTED, "Nested namespace is not enabled for this catalog");
         }
-        if (!parentNamespace.isEmpty()) {
-            schemaName = parentNamespace + NAMESPACE_SEPARATOR + schemaName;
-        }
         return Namespace.of(Splitter.on(NAMESPACE_SEPARATOR).omitEmptyStrings().trimResults().splitToList(schemaName).toArray(new String[0]));
     }
 
     private String toSchemaName(Namespace namespace)
     {
-        if (this.parentNamespace.isEmpty()) {
-            if (!nestedNamespaceEnabled && namespace.length() != 1) {
-                throw new TrinoException(NOT_SUPPORTED, "Nested namespace is not enabled for this catalog");
-            }
-            return namespace.toString();
-        }
-        if (!nestedNamespaceEnabled && ((namespace.length() - parentNamespace.length()) > 1)) {
+        if (!nestedNamespaceEnabled && namespace.length() != 1) {
             throw new TrinoException(NOT_SUPPORTED, "Nested namespace is not enabled for this catalog");
         }
-        return Arrays.stream(namespace.levels(), this.parentNamespace.length(), namespace.length())
-                .collect(joining(NAMESPACE_SEPARATOR));
+        return String.join(NAMESPACE_SEPARATOR, namespace.levels());
     }
 
     private TableIdentifier toIdentifier(SchemaTableName schemaTableName)
diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/IcebergQueryRunner.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/IcebergQueryRunner.java
@@ -259,7 +259,7 @@ public static void main(String[] args)
                     .addIcebergProperty("iceberg.security", "read_only")
                     .addIcebergProperty("iceberg.catalog.type", "rest")
                     .addIcebergProperty("iceberg.rest-catalog.uri", unityCatalog.uri() + "/iceberg")
-                    .addIcebergProperty("iceberg.rest-catalog.parent-namespace", "unity")
+                    .addIcebergProperty("iceberg.rest-catalog.warehouse", "unity")
                     .addIcebergProperty("iceberg.register-table-procedure.enabled", "true")
                     .disableSchemaInitializer()
                     .build();
diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestIcebergRestCatalogConfig.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestIcebergRestCatalogConfig.java
@@ -33,7 +33,6 @@ public void testDefaults()
                 .setBaseUri(null)
                 .setPrefix(null)
                 .setWarehouse(null)
-                .setParentNamespace(null)
                 .setNestedNamespaceEnabled(false)
                 .setSessionType(IcebergRestCatalogConfig.SessionType.NONE)
                 .setSecurity(IcebergRestCatalogConfig.Security.NONE)
@@ -49,7 +48,6 @@ public void testExplicitPropertyMappings()
                 .put("iceberg.rest-catalog.uri", "http://localhost:1234")
                 .put("iceberg.rest-catalog.prefix", "dev")
                 .put("iceberg.rest-catalog.warehouse", "test_warehouse_identifier")
-                .put("iceberg.rest-catalog.parent-namespace", "main")
                 .put("iceberg.rest-catalog.nested-namespace-enabled", "true")
                 .put("iceberg.rest-catalog.security", "OAUTH2")
                 .put("iceberg.rest-catalog.session", "USER")
@@ -62,7 +60,6 @@ public void testExplicitPropertyMappings()
                 .setBaseUri("http://localhost:1234")
                 .setPrefix("dev")
                 .setWarehouse("test_warehouse_identifier")
-                .setParentNamespace("main")
                 .setNestedNamespaceEnabled(true)
                 .setSessionType(IcebergRestCatalogConfig.SessionType.USER)
                 .setSecurity(IcebergRestCatalogConfig.Security.OAUTH2)
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
@@ -33,7 +33,6 @@
 import io.trino.spi.security.TrinoPrincipal;
 import io.trino.spi.type.TestingTypeManager;
 import io.trino.spi.type.VarcharType;
-import org.apache.iceberg.catalog.Namespace;
 import org.apache.iceberg.exceptions.BadRequestException;
 import org.apache.iceberg.rest.DelegatingRestSessionCatalog;
 import org.apache.iceberg.rest.RESTSessionCatalog;
@@ -87,7 +86,6 @@ private static TrinoRestCatalog createTrinoRestCatalog(boolean useUniqueTableLoc
                 new CatalogName(catalogName),
                 NONE,
                 ImmutableMap.of(),
-                Namespace.empty(),
                 false,
                 "test",
                 new TestingTypeManager(),
