From ae16c69b04579fbea6d2c8881b3b28c0ade2c4f2 Mon Sep 17 00:00:00 2001 From: Denodo Research Labs Date: Fri, 15 Nov 2024 14:02:21 +0100 Subject: [PATCH] Add property to allow querying in nested namespaces The configuration property iceberg.rest.nested.namespace.enabled allows nested namespace in Iceberg's REST Catalog --- .../src/main/sphinx/connector/iceberg.rst | 5 ++ .../iceberg/IcebergNativeCatalogFactory.java | 5 ++ .../presto/iceberg/IcebergNativeMetadata.java | 47 ++++++++++++------- .../facebook/presto/iceberg/IcebergUtil.java | 4 +- .../rest/IcebergRestCatalogFactory.java | 8 ++++ .../iceberg/rest/IcebergRestConfig.java | 14 ++++++ .../util/IcebergPrestoModelConverters.java | 28 ++++++++--- .../iceberg/rest/TestIcebergRestConfig.java | 7 ++- .../TestIcebergSmokeRestNestedNamespace.java | 34 +++++++++++++- 9 files changed, 123 insertions(+), 29 deletions(-) diff --git a/presto-docs/src/main/sphinx/connector/iceberg.rst b/presto-docs/src/main/sphinx/connector/iceberg.rst index 86f83bb3674c6..504a37d31dfb2 100644 --- a/presto-docs/src/main/sphinx/connector/iceberg.rst +++ b/presto-docs/src/main/sphinx/connector/iceberg.rst @@ -224,6 +224,11 @@ Property Name Description ``iceberg.rest.auth.oauth2.credential``. Example: ``PRINCIPAL_ROLE:ALL`` +``iceberg.rest.nested.namespace.enabled`` In REST Catalogs, tables are grouped into namespaces, that can be + nested. But if a large number of recursive namespaces result in + lower performance, querying nested namespaces can be disabled. + Defaults to ``true``. + ``iceberg.rest.session.type`` The session type to use when communicating with the REST catalog. Available values are ``NONE`` or ``USER`` (default: ``NONE``). diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergNativeCatalogFactory.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergNativeCatalogFactory.java index d0a361184af13..4d23a97a3ff71 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergNativeCatalogFactory.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergNativeCatalogFactory.java @@ -99,6 +99,11 @@ public SupportsNamespaces getNamespaces(ConnectorSession session) throw new PrestoException(NOT_SUPPORTED, "Iceberg catalog of type " + catalogType + " does not support namespace operations"); } + public boolean isNestedNamespaceEnabled() + { + return false; + } + protected String getCacheKey(ConnectorSession session) { StringBuilder sb = new StringBuilder(); diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergNativeMetadata.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergNativeMetadata.java index 76e479e3f35fe..a29c877b22f77 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergNativeMetadata.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergNativeMetadata.java @@ -73,6 +73,7 @@ import static com.facebook.presto.iceberg.util.IcebergPrestoModelConverters.toIcebergNamespace; import static com.facebook.presto.iceberg.util.IcebergPrestoModelConverters.toIcebergTableIdentifier; import static com.facebook.presto.iceberg.util.IcebergPrestoModelConverters.toPrestoSchemaName; +import static com.facebook.presto.iceberg.util.IcebergPrestoModelConverters.toPrestoSchemaTableName; import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; import static com.facebook.presto.spi.StandardErrorCode.SCHEMA_NOT_EMPTY; import static com.google.common.base.Verify.verify; @@ -139,7 +140,14 @@ protected boolean tableExists(ConnectorSession session, SchemaTableName schemaTa public List listSchemaNames(ConnectorSession session) { SupportsNamespaces supportsNamespaces = catalogFactory.getNamespaces(session); - return listNestedNamespaces(supportsNamespaces, Namespace.empty()); + if (catalogFactory.isNestedNamespaceEnabled()) { + return listNestedNamespaces(supportsNamespaces, Namespace.empty()); + } + + return supportsNamespaces.listNamespaces() + .stream() + .map(IcebergPrestoModelConverters::toPrestoSchemaName) + .collect(toList()); } private List listNestedNamespaces(SupportsNamespaces supportsNamespaces, Namespace parentNamespace) @@ -161,16 +169,16 @@ public List listTables(ConnectorSession session, Optional toPrestoSchemaTableName(tableIdentifier, catalogFactory.isNestedNamespaceEnabled())) .collect(toList()); } @Override public void createSchema(ConnectorSession session, String schemaName, Map properties) { - catalogFactory.getNamespaces(session).createNamespace(toIcebergNamespace(Optional.of(schemaName)), + catalogFactory.getNamespaces(session).createNamespace(toIcebergNamespace(Optional.of(schemaName), catalogFactory.isNestedNamespaceEnabled()), properties.entrySet().stream() .collect(toMap(Map.Entry::getKey, e -> e.getValue().toString()))); } @@ -179,7 +187,7 @@ public void createSchema(ConnectorSession session, String schemaName, Map listViews(ConnectorSession session, Optional getViews(ConnectorSession s for (SchemaTableName schemaTableName : tableNames) { try { - if (((ViewCatalog) catalog).viewExists(toIcebergTableIdentifier(schemaTableName))) { - View view = ((ViewCatalog) catalog).loadView(toIcebergTableIdentifier(schemaTableName)); + TableIdentifier viewIdentifier = toIcebergTableIdentifier(schemaTableName, catalogFactory.isNestedNamespaceEnabled()); + if (((ViewCatalog) catalog).viewExists(viewIdentifier)) { + View view = ((ViewCatalog) catalog).loadView(viewIdentifier); verifyAndPopulateViews(view, schemaTableName, view.sqlFor(VIEW_DIALECT).sql(), views); } } @@ -277,7 +287,7 @@ public void dropView(ConnectorSession session, SchemaTableName viewName) if (!(catalog instanceof ViewCatalog)) { throw new PrestoException(NOT_SUPPORTED, "This connector does not support dropping views"); } - ((ViewCatalog) catalog).dropView(toIcebergTableIdentifier(viewName)); + ((ViewCatalog) catalog).dropView(toIcebergTableIdentifier(viewName, catalogFactory.isNestedNamespaceEnabled())); } private void verifyAndPopulateViews(View view, SchemaTableName schemaTableName, String viewData, ImmutableMap.Builder views) @@ -304,7 +314,10 @@ public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, Con try { transaction = catalogFactory.getCatalog(session).newCreateTableTransaction( - toIcebergTableIdentifier(schemaTableName), schema, partitionSpec, populateTableProperties(tableMetadata, fileFormat, session)); + toIcebergTableIdentifier(schemaTableName, catalogFactory.isNestedNamespaceEnabled()), + schema, + partitionSpec, + populateTableProperties(tableMetadata, fileFormat, session)); } catch (AlreadyExistsException e) { throw new TableAlreadyExistsException(schemaTableName); @@ -328,7 +341,7 @@ public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle { IcebergTableHandle icebergTableHandle = (IcebergTableHandle) tableHandle; verify(icebergTableHandle.getIcebergTableName().getTableType() == DATA, "only the data table can be dropped"); - TableIdentifier tableIdentifier = toIcebergTableIdentifier(icebergTableHandle.getSchemaTableName()); + TableIdentifier tableIdentifier = toIcebergTableIdentifier(icebergTableHandle.getSchemaTableName(), catalogFactory.isNestedNamespaceEnabled()); catalogFactory.getCatalog(session).dropTable(tableIdentifier); } @@ -337,20 +350,20 @@ public void renameTable(ConnectorSession session, ConnectorTableHandle tableHand { IcebergTableHandle icebergTableHandle = (IcebergTableHandle) tableHandle; verify(icebergTableHandle.getIcebergTableName().getTableType() == DATA, "only the data table can be renamed"); - TableIdentifier from = toIcebergTableIdentifier(icebergTableHandle.getSchemaTableName()); - TableIdentifier to = toIcebergTableIdentifier(newTable); + TableIdentifier from = toIcebergTableIdentifier(icebergTableHandle.getSchemaTableName(), catalogFactory.isNestedNamespaceEnabled()); + TableIdentifier to = toIcebergTableIdentifier(newTable, catalogFactory.isNestedNamespaceEnabled()); catalogFactory.getCatalog(session).renameTable(from, to); } @Override public void registerTable(ConnectorSession clientSession, SchemaTableName schemaTableName, Path metadataLocation) { - catalogFactory.getCatalog(clientSession).registerTable(toIcebergTableIdentifier(schemaTableName), metadataLocation.toString()); + catalogFactory.getCatalog(clientSession).registerTable(toIcebergTableIdentifier(schemaTableName, catalogFactory.isNestedNamespaceEnabled()), metadataLocation.toString()); } @Override public void unregisterTable(ConnectorSession clientSession, SchemaTableName schemaTableName) { - catalogFactory.getCatalog(clientSession).dropTable(toIcebergTableIdentifier(schemaTableName), false); + catalogFactory.getCatalog(clientSession).dropTable(toIcebergTableIdentifier(schemaTableName, catalogFactory.isNestedNamespaceEnabled()), false); } } diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergUtil.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergUtil.java index bb3139fba3e67..95fcc4fd0d497 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergUtil.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergUtil.java @@ -254,7 +254,7 @@ public static Table getHiveIcebergTable(ExtendedHiveMetastore metastore, HdfsEnv public static Table getNativeIcebergTable(IcebergNativeCatalogFactory catalogFactory, ConnectorSession session, SchemaTableName table) { - return catalogFactory.getCatalog(session).loadTable(toIcebergTableIdentifier(table)); + return catalogFactory.getCatalog(session).loadTable(toIcebergTableIdentifier(table, catalogFactory.isNestedNamespaceEnabled())); } public static View getNativeIcebergView(IcebergNativeCatalogFactory catalogFactory, ConnectorSession session, SchemaTableName table) @@ -263,7 +263,7 @@ public static View getNativeIcebergView(IcebergNativeCatalogFactory catalogFacto if (!(catalog instanceof ViewCatalog)) { throw new PrestoException(NOT_SUPPORTED, "This connector does not support get views"); } - return ((ViewCatalog) catalog).loadView(toIcebergTableIdentifier(table)); + return ((ViewCatalog) catalog).loadView(toIcebergTableIdentifier(table, catalogFactory.isNestedNamespaceEnabled())); } public static List getPartitionKeyColumnHandles(IcebergTableHandle tableHandle, Table table, TypeManager typeManager) diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestCatalogFactory.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestCatalogFactory.java index ca363e793ace3..3f4d9422d0146 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestCatalogFactory.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestCatalogFactory.java @@ -59,6 +59,7 @@ public class IcebergRestCatalogFactory private final IcebergRestConfig catalogConfig; private final NodeVersion nodeVersion; private final String catalogName; + private final boolean nestedNamespaceEnabled; @Inject public IcebergRestCatalogFactory( @@ -73,6 +74,7 @@ public IcebergRestCatalogFactory( this.catalogConfig = requireNonNull(catalogConfig, "catalogConfig is null"); this.nodeVersion = requireNonNull(nodeVersion, "nodeVersion is null"); this.catalogName = requireNonNull(catalogName, "catalogName is null").getCatalogName(); + this.nestedNamespaceEnabled = catalogConfig.isNestedNamespaceEnabled(); } @Override @@ -135,6 +137,12 @@ protected Map getCatalogProperties(ConnectorSession session) return properties.build(); } + @Override + public boolean isNestedNamespaceEnabled() + { + return this.nestedNamespaceEnabled; + } + protected SessionContext convertSession(ConnectorSession session) { RestSessionBuilder sessionContextBuilder = catalogConfig.getSessionType() diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestConfig.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestConfig.java index e46dfda2109c9..613a48d02f13a 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestConfig.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestConfig.java @@ -29,6 +29,7 @@ public class IcebergRestConfig private String credential; private String token; private String scope; + private boolean nestedNamespaceEnabled = true; @NotNull public Optional getServerUri() @@ -122,6 +123,19 @@ public IcebergRestConfig setScope(String scope) return this; } + public boolean isNestedNamespaceEnabled() + { + return nestedNamespaceEnabled; + } + + @Config("iceberg.rest.nested.namespace.enabled") + @ConfigDescription("Allows querying nested namespaces. Default: true") + public IcebergRestConfig setNestedNamespaceEnabled(boolean nestedNamespaceEnabled) + { + this.nestedNamespaceEnabled = nestedNamespaceEnabled; + return this; + } + public boolean credentialOrTokenExists() { return credential != null || token != null; diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/util/IcebergPrestoModelConverters.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/util/IcebergPrestoModelConverters.java index 7c8dc9fe5922b..efb830185af78 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/util/IcebergPrestoModelConverters.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/util/IcebergPrestoModelConverters.java @@ -13,6 +13,7 @@ */ package com.facebook.presto.iceberg.util; +import com.facebook.presto.spi.PrestoException; import com.facebook.presto.spi.SchemaTableName; import com.google.common.base.Splitter; import org.apache.iceberg.catalog.Namespace; @@ -20,6 +21,9 @@ import java.util.Optional; +import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; +import static java.lang.String.format; + public class IcebergPrestoModelConverters { private static final String NAMESPACE_SEPARATOR = "."; @@ -34,26 +38,38 @@ public static String toPrestoSchemaName(Namespace icebergNamespace) return icebergNamespace.toString(); } - public static Namespace toIcebergNamespace(Optional prestoSchemaName) + public static Namespace toIcebergNamespace(Optional prestoSchemaName, boolean nestedNamespaceEnabled) { if (prestoSchemaName.isPresent()) { + checkNestedNamespaceSupport(prestoSchemaName.get(), nestedNamespaceEnabled); return Namespace.of(NAMESPACE_SPLITTER.splitToList(prestoSchemaName.get()).toArray(new String[0])); } return Namespace.empty(); } - public static SchemaTableName toPrestoSchemaTableName(TableIdentifier icebergTableIdentifier) + public static SchemaTableName toPrestoSchemaTableName(TableIdentifier icebergTableIdentifier, boolean nestedNamespaceEnabled) { - return new SchemaTableName(icebergTableIdentifier.namespace().toString(), icebergTableIdentifier.name()); + String schemaName = icebergTableIdentifier.namespace().toString(); + checkNestedNamespaceSupport(schemaName, nestedNamespaceEnabled); + return new SchemaTableName(schemaName, icebergTableIdentifier.name()); } - public static TableIdentifier toIcebergTableIdentifier(SchemaTableName prestoSchemaTableName) + public static TableIdentifier toIcebergTableIdentifier(SchemaTableName prestoSchemaTableName, boolean nestedNamespaceEnabled) { - return toIcebergTableIdentifier(toIcebergNamespace(Optional.ofNullable(prestoSchemaTableName.getSchemaName())), prestoSchemaTableName.getTableName()); + return toIcebergTableIdentifier(toIcebergNamespace(Optional.ofNullable(prestoSchemaTableName.getSchemaName()), nestedNamespaceEnabled), + prestoSchemaTableName.getTableName(), nestedNamespaceEnabled); } - public static TableIdentifier toIcebergTableIdentifier(Namespace icebergNamespace, String prestoTableName) + public static TableIdentifier toIcebergTableIdentifier(Namespace icebergNamespace, String prestoTableName, boolean nestedNamespaceEnabled) { + checkNestedNamespaceSupport(icebergNamespace.toString(), nestedNamespaceEnabled); return TableIdentifier.of(icebergNamespace, prestoTableName); } + + private static void checkNestedNamespaceSupport(String schemaName, boolean nestedNamespaceEnabled) + { + if (!nestedNamespaceEnabled && schemaName.contains(NAMESPACE_SEPARATOR)) { + throw new PrestoException(NOT_SUPPORTED, format("Nested namespaces are disabled. Schema %s is not valid", schemaName)); + } + } } diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergRestConfig.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergRestConfig.java index e21f70992e57c..e500a429e1b61 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergRestConfig.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergRestConfig.java @@ -36,7 +36,8 @@ public void testDefaults() .setCredential(null) .setToken(null) .setScope(null) - .setSessionType(null)); + .setSessionType(null) + .setNestedNamespaceEnabled(true)); } @Test @@ -50,6 +51,7 @@ public void testExplicitPropertyMappings() .put("iceberg.rest.auth.oauth2.token", "SXVLUXUhIExFQ0tFUiEK") .put("iceberg.rest.auth.oauth2.scope", "PRINCIPAL_ROLE:ALL") .put("iceberg.rest.session.type", "USER") + .put("iceberg.rest.nested.namespace.enabled", "false") .build(); IcebergRestConfig expected = new IcebergRestConfig() @@ -59,7 +61,8 @@ public void testExplicitPropertyMappings() .setCredential("key:secret") .setToken("SXVLUXUhIExFQ0tFUiEK") .setScope("PRINCIPAL_ROLE:ALL") - .setSessionType(USER); + .setSessionType(USER) + .setNestedNamespaceEnabled(false); assertFullMapping(properties, expected); } diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRestNestedNamespace.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRestNestedNamespace.java index 0e376eeed8e39..6660515ae6811 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRestNestedNamespace.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRestNestedNamespace.java @@ -29,6 +29,7 @@ import com.facebook.presto.spi.SchemaTableName; import com.facebook.presto.testing.MaterializedResult; import com.facebook.presto.testing.QueryRunner; +import com.facebook.presto.tests.DistributedQueryRunner; import com.google.common.collect.ImmutableMap; import org.apache.iceberg.Table; import org.assertj.core.util.Files; @@ -38,6 +39,7 @@ import org.testng.annotations.Test; import java.io.File; +import java.util.Map; import java.util.Optional; import java.util.OptionalInt; @@ -60,6 +62,8 @@ public class TestIcebergSmokeRestNestedNamespace extends TestIcebergSmokeRest { + private static final String ICEBERG_NESTED_NAMESPACE_DISABLED_CATALOG = "iceberg_without_nested_namespaces"; + private File warehouseLocation; private TestingHttpServer restServer; private String serverUri; @@ -105,9 +109,10 @@ protected String getLocation(String schema, String table) protected QueryRunner createQueryRunner() throws Exception { - return IcebergQueryRunner.createIcebergQueryRunner( + Map restConnectorProperties = restConnectorProperties(serverUri); + DistributedQueryRunner icebergQueryRunner = IcebergQueryRunner.createIcebergQueryRunner( ImmutableMap.of(), - restConnectorProperties(serverUri), + restConnectorProperties, PARQUET, true, false, @@ -116,6 +121,15 @@ protected QueryRunner createQueryRunner() Optional.of(warehouseLocation.toPath()), false, Optional.of("ns1.ns2")); + + // additional catalog for testing nested namespace disabled + icebergQueryRunner.createCatalog(ICEBERG_NESTED_NAMESPACE_DISABLED_CATALOG, "iceberg", + new ImmutableMap.Builder() + .putAll(restConnectorProperties) + .put("iceberg.rest.nested.namespace.enabled", "false") + .build()); + + return icebergQueryRunner; } protected IcebergNativeCatalogFactory getCatalogFactory(IcebergRestConfig restConfig) @@ -331,4 +345,20 @@ public void testView() " orders", schemaName)); assertUpdate(session, "DROP VIEW view_orders"); } + + @Test + void testNestedNamespaceDisabled() + { + assertQuery(format("SHOW SCHEMAS FROM %s", ICEBERG_NESTED_NAMESPACE_DISABLED_CATALOG), + "VALUES 'ns1', 'tpch', 'tpcds', 'information_schema'"); + + assertQueryFails(format("CREATE SCHEMA %s.\"ns1.ns2.ns3\"", ICEBERG_NESTED_NAMESPACE_DISABLED_CATALOG), + "Nested namespaces are disabled. Schema ns1.ns2.ns3 is not valid"); + assertQueryFails(format("CREATE TABLE %s.\"ns1.ns2\".test_table(a int)", ICEBERG_NESTED_NAMESPACE_DISABLED_CATALOG), + "Nested namespaces are disabled. Schema ns1.ns2 is not valid"); + assertQueryFails(format("SELECT * FROM %s.\"ns1.ns2\".orders", ICEBERG_NESTED_NAMESPACE_DISABLED_CATALOG), + "Nested namespaces are disabled. Schema ns1.ns2 is not valid"); + assertQueryFails(format("SHOW TABLES IN %s.\"ns1.ns2\"", ICEBERG_NESTED_NAMESPACE_DISABLED_CATALOG), + "line 1:1: Schema 'ns1.ns2' does not exist"); + } }