From e8cba2fc47551fc1b583d758ff62b5018d6b2e18 Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Fri, 30 May 2025 16:21:18 +0200 Subject: [PATCH 1/5] Overriding metadata existence check --- .../catalog/iceberg/IcebergCatalog.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index ba9df38c96..a5ec623b14 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -212,6 +212,31 @@ public IcebergCatalog( this.polarisEventListener = polarisEventListener; } + @Override + public boolean tableExists(TableIdentifier identifier) { + try { + loadTableIgnoreMetadata(identifier); + return true; + } catch (NoSuchTableException e) { + return false; + } + } + + public Table loadTableIgnoreMetadata(TableIdentifier identifier) { + Table result; + if (isValidIdentifier(identifier)) { + TableOperations ops = newTableOps(identifier); + if (ops.current() == null) { + throw new NoSuchTableException("Table does not exist: %s", identifier); + } else { + result = new BaseTable(ops, fullTableName(name(), identifier), metricsReporter()); + } + } else { + throw new NoSuchTableException("Invalid table identifier: %s", identifier); + } + return result; + } + @Override public String name() { return catalogName; From 5cadeb66aedcdf962b8cbc964f742ad7a7d37fc5 Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Fri, 30 May 2025 16:24:45 +0200 Subject: [PATCH 2/5] Simplified code --- .../catalog/iceberg/IcebergCatalog.java | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index a5ec623b14..8757578dec 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -214,27 +214,10 @@ public IcebergCatalog( @Override public boolean tableExists(TableIdentifier identifier) { - try { - loadTableIgnoreMetadata(identifier); - return true; - } catch (NoSuchTableException e) { - return false; - } - } - - public Table loadTableIgnoreMetadata(TableIdentifier identifier) { - Table result; if (isValidIdentifier(identifier)) { - TableOperations ops = newTableOps(identifier); - if (ops.current() == null) { - throw new NoSuchTableException("Table does not exist: %s", identifier); - } else { - result = new BaseTable(ops, fullTableName(name(), identifier), metricsReporter()); - } - } else { - throw new NoSuchTableException("Invalid table identifier: %s", identifier); + return newTableOps(identifier).current() != null; } - return result; + return false; } @Override From 8d6c0e3a3a58a7d1564785e4cde6ae3b7c38a6ac Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Mon, 2 Jun 2025 11:29:49 +0200 Subject: [PATCH 3/5] Added test --- .../PolarisRestCatalogIntegrationTest.java | 28 +++++++++++++++---- .../catalog/iceberg/IcebergCatalog.java | 2 +- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index ae42c5ae5c..acbfbe04b6 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -36,12 +36,7 @@ import java.lang.reflect.Method; import java.net.URI; import java.nio.file.Path; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.UUID; +import java.util.*; import java.util.stream.Stream; import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.BaseTable; @@ -67,6 +62,7 @@ import org.apache.iceberg.rest.RESTUtil; import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.responses.ErrorResponse; +import org.apache.iceberg.types.Type; import org.apache.iceberg.types.Types; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.Catalog; @@ -562,6 +558,26 @@ public void testCreateTableWithOverriddenBaseLocationCannotOverlapSibling() { .isInstanceOf(ForbiddenException.class) .hasMessageContaining("because it conflicts with existing table or namespace"); } + + @Test + public void testCreateTableWithMetadataConflictingName() { + Catalog catalog = managementApi.getCatalog(currentCatalogName); + Map catalogProps = new HashMap<>(catalog.getProperties().toMap()); + catalogProps.put( + FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false"); + managementApi.updateCatalog(catalog, catalogProps); + + restCatalog.createNamespace(Namespace.of("ns1")); + TableIdentifier tableIdentifier = TableIdentifier.of("ns1", "history"); + restCatalog + .buildTable(tableIdentifier, SCHEMA) + .withProperty("stage-create", "true") + .create(); + Table table = restCatalog.loadTable(tableIdentifier); + assertThat(table) + .isNotNull() + .isInstanceOf(BaseTable.class); + } @Test public void testCreateTableWithOverriddenBaseLocationMustResideInNsDirectory() { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 8757578dec..6098b61c2b 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -215,7 +215,7 @@ public IcebergCatalog( @Override public boolean tableExists(TableIdentifier identifier) { if (isValidIdentifier(identifier)) { - return newTableOps(identifier).current() != null; + return newTableOps(identifier).current() != null; } return false; } From ebb7e5e54f45a707e2401e12b61f8e256d93a694 Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Mon, 2 Jun 2025 11:31:35 +0200 Subject: [PATCH 4/5] Spotless --- .../it/test/PolarisRestCatalogIntegrationTest.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index acbfbe04b6..ca553ea58d 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -62,7 +62,6 @@ import org.apache.iceberg.rest.RESTUtil; import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.responses.ErrorResponse; -import org.apache.iceberg.types.Type; import org.apache.iceberg.types.Types; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.Catalog; @@ -558,25 +557,20 @@ public void testCreateTableWithOverriddenBaseLocationCannotOverlapSibling() { .isInstanceOf(ForbiddenException.class) .hasMessageContaining("because it conflicts with existing table or namespace"); } - + @Test public void testCreateTableWithMetadataConflictingName() { Catalog catalog = managementApi.getCatalog(currentCatalogName); Map catalogProps = new HashMap<>(catalog.getProperties().toMap()); catalogProps.put( - FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false"); + FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false"); managementApi.updateCatalog(catalog, catalogProps); restCatalog.createNamespace(Namespace.of("ns1")); TableIdentifier tableIdentifier = TableIdentifier.of("ns1", "history"); - restCatalog - .buildTable(tableIdentifier, SCHEMA) - .withProperty("stage-create", "true") - .create(); + restCatalog.buildTable(tableIdentifier, SCHEMA).withProperty("stage-create", "true").create(); Table table = restCatalog.loadTable(tableIdentifier); - assertThat(table) - .isNotNull() - .isInstanceOf(BaseTable.class); + assertThat(table).isNotNull().isInstanceOf(BaseTable.class); } @Test From cd47fcf24a7e286f9f8530caf84437e5047d931e Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Mon, 2 Jun 2025 14:17:22 +0200 Subject: [PATCH 5/5] Removed not-required property --- .../service/it/test/PolarisRestCatalogIntegrationTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index ca553ea58d..e76aa32f74 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -561,11 +561,6 @@ public void testCreateTableWithOverriddenBaseLocationCannotOverlapSibling() { @Test public void testCreateTableWithMetadataConflictingName() { Catalog catalog = managementApi.getCatalog(currentCatalogName); - Map catalogProps = new HashMap<>(catalog.getProperties().toMap()); - catalogProps.put( - FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false"); - managementApi.updateCatalog(catalog, catalogProps); - restCatalog.createNamespace(Namespace.of("ns1")); TableIdentifier tableIdentifier = TableIdentifier.of("ns1", "history"); restCatalog.buildTable(tableIdentifier, SCHEMA).withProperty("stage-create", "true").create();