From 4f775a633c93c387eebcbf1513e50a2abfa26c32 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 20 Mar 2025 13:18:34 -0700 Subject: [PATCH 01/23] add missing apis --- .../catalog/generic/GenericTableCatalog.java | 58 ++++++++++++++++++- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index 86c4f0bbfe..5b3cd690fc 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -19,21 +19,35 @@ package org.apache.polaris.service.catalog.generic; import jakarta.ws.rs.core.SecurityContext; + +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; + +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.TableOperations; +import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.BadRequestException; +import org.apache.iceberg.exceptions.ForbiddenException; import org.apache.iceberg.exceptions.NoSuchTableException; +import org.apache.polaris.core.catalog.PolarisCatalogHelpers; +import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.GenericTableEntity; import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.entity.PolarisTaskConstants; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.persistence.dao.entity.BaseResult; +import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.apache.polaris.service.catalog.io.FileIOFactory; @@ -97,7 +111,6 @@ public void createGenericTable( List catalogPath = resolvedParent.getRawFullPath(); - // TODO we need to filter by type here? PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getPassthroughResolvedPath( tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); @@ -146,7 +159,6 @@ public void createGenericTable( } public GenericTableEntity loadGenericTable(TableIdentifier tableIdentifier) { - // TODO we need to filter by type here? PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getPassthroughResolvedPath( tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); @@ -159,4 +171,46 @@ public GenericTableEntity loadGenericTable(TableIdentifier tableIdentifier) { return entity; } } + + public boolean dropGenericTable(TableIdentifier tableIdentifier) { + PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getPassthroughResolvedPath( + tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); + + List catalogPath = resolvedEntities.getRawParentPath(); + PolarisEntity leafEntity = resolvedEntities.getRawLeafEntity(); + + DropEntityResult dropEntityResult = this.metaStoreManager + .dropEntityIfExists( + this.callContext.getPolarisCallContext(), + PolarisEntity.toCoreList(catalogPath), + leafEntity, + Map.of(), + false); + + if (!dropEntityResult.isSuccess()) { + if (dropEntityResult.failedBecauseNotEmpty()) { + throw new IllegalStateException("Cannot drop entity because it has children"); + } else { + throw new BadRequestException("Failed to drop " + tableIdentifier.toString()); + } + } + return true; + } + + public List listGenericTables(Namespace namespace) { + PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); + + List catalogPath = resolvedEntities.getRawFullPath(); + List entities = + PolarisEntity.toNameAndIdList( + this.metaStoreManager + .listEntities( + this.callContext.getPolarisCallContext(), + PolarisEntity.toCoreList(catalogPath), + PolarisEntityType.GENERIC_TABLE, + PolarisEntitySubType.ANY_SUBTYPE) + .getEntities()); + return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities); + + } } From e2fcd5e5eb26f0983ddf99a2f4a966e4f6c1bc1e Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 20 Mar 2025 20:55:01 -0700 Subject: [PATCH 02/23] more tests, fixes --- .../catalog/GenericTableCatalogTest.java | 200 +++++++++++++++++- .../catalog/generic/GenericTableCatalog.java | 26 +-- 2 files changed, 206 insertions(+), 20 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java index 57d49226c4..e1161694f8 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java @@ -18,12 +18,15 @@ */ package org.apache.polaris.service.quarkus.catalog; +import static org.apache.iceberg.types.Types.NestedField.required; import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.when; +import com.google.common.collect.ImmutableMap; import io.quarkus.test.junit.QuarkusMock; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.QuarkusTestProfile; +import io.quarkus.test.junit.TestProfile; import jakarta.inject.Inject; import jakarta.ws.rs.core.SecurityContext; import java.io.IOException; @@ -34,8 +37,11 @@ import java.util.Set; import java.util.function.Supplier; import org.apache.commons.lang3.NotImplementedException; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.Schema; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.types.Types; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDiagnostics; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; @@ -52,7 +58,6 @@ import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PrincipalEntity; -import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; @@ -60,6 +65,7 @@ import org.apache.polaris.core.persistence.cache.EntityCache; import org.apache.polaris.core.persistence.dao.entity.BaseResult; import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult; +import org.apache.polaris.core.persistence.transactional.TransactionalPersistence; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; import org.apache.polaris.core.storage.aws.AwsCredentialsStorageIntegration; @@ -69,7 +75,9 @@ import org.apache.polaris.service.catalog.PolarisPassthroughResolutionView; import org.apache.polaris.service.catalog.generic.GenericTableCatalog; import org.apache.polaris.service.catalog.iceberg.IcebergCatalog; +import org.apache.polaris.service.catalog.io.DefaultFileIOFactory; import org.apache.polaris.service.catalog.io.FileIOFactory; +import org.apache.polaris.service.config.RealmEntityManagerFactory; import org.apache.polaris.service.storage.PolarisStorageIntegrationProviderImpl; import org.apache.polaris.service.task.TaskExecutor; import org.assertj.core.api.Assertions; @@ -85,12 +93,20 @@ import software.amazon.awssdk.services.sts.model.Credentials; @QuarkusTest +@TestProfile(GenericTableCatalogTest.Profile.class) public class GenericTableCatalogTest { + public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { - return Map.of(); + return Map.of( + "polaris.features.defaults.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", + "true", + "polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", + "true", + "polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", + "[\"FILE\"]"); } } @@ -120,6 +136,11 @@ public Map getConfigOverrides() { private PolarisEntity catalogEntity; private SecurityContext securityContext; + protected static final Schema SCHEMA = + new Schema( + required(3, "id", Types.IntegerType.get(), "unique ID 🤪"), + required(4, "data", Types.StringType.get())); + @BeforeAll public static void setUpMocks() { PolarisStorageIntegrationProviderImpl mock = @@ -199,6 +220,10 @@ public void before(TestInfo testInfo) { new PolarisPassthroughResolutionView( callContext, entityManager, securityContext, CATALOG_NAME); TaskExecutor taskExecutor = Mockito.mock(); + RealmEntityManagerFactory realmEntityManagerFactory = + new RealmEntityManagerFactory(createMockMetaStoreManagerFactory()); + this.fileIOFactory = + new DefaultFileIOFactory(realmEntityManagerFactory, managerFactory, configurationStore); StsClient stsClient = Mockito.mock(StsClient.class); when(stsClient.assumeRole(isA(AssumeRoleRequest.class))) @@ -235,6 +260,10 @@ public void before(TestInfo testInfo) { securityContext, taskExecutor, fileIOFactory); + this.icebergCatalog.initialize( + CATALOG_NAME, + ImmutableMap.of( + CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO")); } @AfterEach @@ -250,8 +279,9 @@ public PolarisMetaStoreManager getOrCreateMetaStoreManager(RealmContext realmCon } @Override - public Supplier getOrCreateSessionSupplier(RealmContext realmContext) { - return () -> polarisContext.getMetaStore(); + public Supplier getOrCreateSessionSupplier( + RealmContext realmContext) { + return () -> ((TransactionalPersistence) polarisContext.getMetaStore()); } @Override @@ -306,4 +336,166 @@ public void testGenericTableRoundTrip() { Assertions.assertThat(resultEntity.getPropertiesAsMap()).isEqualTo(properties); Assertions.assertThat(resultEntity.getName()).isEqualTo(tableName); } + + @Test + public void testLoadNonExistentTable() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + Assertions.assertThatCode( + () -> genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", "t1"))) + .hasMessageContaining("does not exist: ns.t1"); + } + + @Test + public void testReadIcebergTableAsGeneric() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + String tableName = "t1"; + + icebergCatalog.createTable(TableIdentifier.of("ns", tableName), SCHEMA); + Assertions.assertThatCode( + () -> genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", tableName))) + .hasMessageContaining("does not exist: ns.t1"); + } + + @Test + public void testReadIcebergViewAsGeneric() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + String tableName = "t1"; + + icebergCatalog.buildView(TableIdentifier.of("ns", tableName)); + Assertions.assertThatCode( + () -> genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", tableName))) + .hasMessageContaining("does not exist: ns.t1"); + } + + @Test + public void testReadGenericAsIcebergTable() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + String tableName = "t1"; + + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), "format", Map.of()); + Assertions.assertThatCode(() -> icebergCatalog.loadTable(TableIdentifier.of("ns", tableName))) + .hasMessageContaining("does not exist: ns.t1"); + } + + @Test + public void testReadGenericAsIcebergView() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + String tableName = "t1"; + + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", tableName), "format", Map.of()); + Assertions.assertThatCode(() -> icebergCatalog.loadView(TableIdentifier.of("ns", tableName))) + .hasMessageContaining("does not exist: ns.t1"); + } + + @Test + public void testListTables() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + for (int i = 0; i < 10; i++) { + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t" + i), "format", Map.of()); + } + + List listResult = genericTableCatalog.listGenericTables(namespace); + + Assertions.assertThat(listResult.size()).isEqualTo(10); + Assertions.assertThat(listResult.stream().map(TableIdentifier::toString).toList()) + .isEqualTo(listResult.stream().map(TableIdentifier::toString).sorted().toList()); + + Assertions.assertThat(icebergCatalog.listTables(namespace)).isEmpty(); + } + + @Test + public void testListTablesEmpty() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + for (int i = 0; i < 10; i++) { + icebergCatalog.createTable(TableIdentifier.of("ns", "t" + i), SCHEMA); + } + + Assertions.assertThat(icebergCatalog.listTables(namespace).size()).isEqualTo(10); + Assertions.assertThat(genericTableCatalog.listGenericTables(namespace)).isEmpty(); + } + + @Test + public void testListIcebergTables() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + for (int i = 0; i < 10; i++) { + icebergCatalog.createTable(TableIdentifier.of("ns", "t" + i), SCHEMA); + } + + List listResult = icebergCatalog.listTables(namespace); + + Assertions.assertThat(listResult.size()).isEqualTo(10); + Assertions.assertThat(listResult.stream().map(TableIdentifier::toString).toList()) + .isEqualTo(listResult.stream().map(TableIdentifier::toString).sorted().toList()); + + Assertions.assertThat(genericTableCatalog.listGenericTables(namespace)).isEmpty(); + } + + @Test + public void testDropNonExistentTable() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + Assertions.assertThatCode( + () -> genericTableCatalog.dropGenericTable(TableIdentifier.of("ns", "t1"))) + .hasMessageContaining("Table does not exist: ns.t1"); + } + + @Test + public void testDropNonExistentNamespace() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + Assertions.assertThatCode( + () -> genericTableCatalog.dropGenericTable(TableIdentifier.of("ns2", "t1"))) + .hasMessageContaining("Table does not exist: ns2.t1"); + } + + @Test + public void testDropIcebergTable() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + icebergCatalog.createTable(TableIdentifier.of("ns", "t1"), SCHEMA); + + Assertions.assertThatCode( + () -> genericTableCatalog.dropGenericTable(TableIdentifier.of("ns", "t1"))) + .hasMessageContaining("Table does not exist: ns.t1"); + } + + @Test + public void testDropViaIceberg() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format", Map.of()); + + Assertions.assertThat(icebergCatalog.dropTable(TableIdentifier.of("ns", "t1"))).isFalse(); + Assertions.assertThat(genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", "t1"))) + .isNotNull(); + } + + @Test + public void testDropViaIcebergView() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format", Map.of()); + + Assertions.assertThat(icebergCatalog.dropView(TableIdentifier.of("ns", "t1"))).isFalse(); + Assertions.assertThat(genericTableCatalog.loadGenericTable(TableIdentifier.of("ns", "t1"))) + .isNotNull(); + } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index 5b3cd690fc..bcfef6c0f5 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -19,30 +19,20 @@ package org.apache.polaris.service.catalog.generic; import jakarta.ws.rs.core.SecurityContext; - -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; - -import org.apache.iceberg.CatalogProperties; -import org.apache.iceberg.TableMetadata; -import org.apache.iceberg.TableOperations; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.BadRequestException; -import org.apache.iceberg.exceptions.ForbiddenException; import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.polaris.core.catalog.PolarisCatalogHelpers; -import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.GenericTableEntity; import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; -import org.apache.polaris.core.entity.PolarisTaskConstants; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; @@ -173,14 +163,19 @@ public GenericTableEntity loadGenericTable(TableIdentifier tableIdentifier) { } public boolean dropGenericTable(TableIdentifier tableIdentifier) { - PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getPassthroughResolvedPath( - tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); + + if (resolvedEntities == null) { + throw new NoSuchTableException("Table does not exist: %s", tableIdentifier); + } List catalogPath = resolvedEntities.getRawParentPath(); PolarisEntity leafEntity = resolvedEntities.getRawLeafEntity(); - DropEntityResult dropEntityResult = this.metaStoreManager - .dropEntityIfExists( + DropEntityResult dropEntityResult = + this.metaStoreManager.dropEntityIfExists( this.callContext.getPolarisCallContext(), PolarisEntity.toCoreList(catalogPath), leafEntity, @@ -191,7 +186,7 @@ public boolean dropGenericTable(TableIdentifier tableIdentifier) { if (dropEntityResult.failedBecauseNotEmpty()) { throw new IllegalStateException("Cannot drop entity because it has children"); } else { - throw new BadRequestException("Failed to drop " + tableIdentifier.toString()); + throw new BadRequestException("Failed to drop"); } } return true; @@ -211,6 +206,5 @@ public List listGenericTables(Namespace namespace) { PolarisEntitySubType.ANY_SUBTYPE) .getEntities()); return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities); - } } From 467ebcb43d87b7402302f6bf9fed1fab4336cf2f Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 21 Mar 2025 00:36:39 -0700 Subject: [PATCH 03/23] clean up drop --- .../service/catalog/generic/GenericTableCatalog.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index bcfef6c0f5..378f15350c 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -162,6 +162,7 @@ public GenericTableEntity loadGenericTable(TableIdentifier tableIdentifier) { } } + @SuppressWarnings("FormatStringAnnotation") public boolean dropGenericTable(TableIdentifier tableIdentifier) { PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getPassthroughResolvedPath( @@ -182,14 +183,7 @@ public boolean dropGenericTable(TableIdentifier tableIdentifier) { Map.of(), false); - if (!dropEntityResult.isSuccess()) { - if (dropEntityResult.failedBecauseNotEmpty()) { - throw new IllegalStateException("Cannot drop entity because it has children"); - } else { - throw new BadRequestException("Failed to drop"); - } - } - return true; + return dropEntityResult.isSuccess(); } public List listGenericTables(Namespace namespace) { From 5f5ca424dd457e0d0f3fcb5c5f3549ee77eab694 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 21 Mar 2025 00:36:42 -0700 Subject: [PATCH 04/23] autolint --- .../polaris/service/catalog/generic/GenericTableCatalog.java | 1 - 1 file changed, 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index 378f15350c..8a5ad65f84 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -24,7 +24,6 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; -import org.apache.iceberg.exceptions.BadRequestException; import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.polaris.core.catalog.PolarisCatalogHelpers; import org.apache.polaris.core.context.CallContext; From 34c8e967c1bbc3b0a4ccb6a160442f3998ad2c79 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 10:13:46 -0700 Subject: [PATCH 05/23] changes per review --- .../quarkus/catalog/GenericTableCatalogTest.java | 14 +++++++++++--- .../catalog/generic/GenericTableCatalog.java | 9 ++++++--- .../service/catalog/iceberg/IcebergCatalog.java | 5 +++-- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java index e1161694f8..62a5a300ff 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java @@ -428,6 +428,14 @@ public void testListTablesEmpty() { Assertions.assertThat(genericTableCatalog.listGenericTables(namespace)).isEmpty(); } + @Test + public void testListTablesNoNamespace() { + Namespace namespace = Namespace.of("ns"); + + Assertions.assertThatCode(() -> genericTableCatalog.listGenericTables(namespace)) + .hasMessageContaining("Namespace"); + } + @Test public void testListIcebergTables() { Namespace namespace = Namespace.of("ns"); @@ -453,7 +461,7 @@ public void testDropNonExistentTable() { Assertions.assertThatCode( () -> genericTableCatalog.dropGenericTable(TableIdentifier.of("ns", "t1"))) - .hasMessageContaining("Table does not exist: ns.t1"); + .hasMessageContaining("Generic table does not exist: ns.t1"); } @Test @@ -463,7 +471,7 @@ public void testDropNonExistentNamespace() { Assertions.assertThatCode( () -> genericTableCatalog.dropGenericTable(TableIdentifier.of("ns2", "t1"))) - .hasMessageContaining("Table does not exist: ns2.t1"); + .hasMessageContaining("Generic table does not exist: ns2.t1"); } @Test @@ -474,7 +482,7 @@ public void testDropIcebergTable() { Assertions.assertThatCode( () -> genericTableCatalog.dropGenericTable(TableIdentifier.of("ns", "t1"))) - .hasMessageContaining("Table does not exist: ns.t1"); + .hasMessageContaining("Generic table does not exist: ns.t1"); } @Test diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index 8a5ad65f84..d284659080 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -24,6 +24,7 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.polaris.core.catalog.PolarisCatalogHelpers; import org.apache.polaris.core.context.CallContext; @@ -155,20 +156,19 @@ public GenericTableEntity loadGenericTable(TableIdentifier tableIdentifier) { GenericTableEntity.of( resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); if (null == entity) { - throw new NoSuchTableException("Table does not exist: %s", tableIdentifier); + throw new NoSuchTableException("Generic table does not exist: %s", tableIdentifier); } else { return entity; } } - @SuppressWarnings("FormatStringAnnotation") public boolean dropGenericTable(TableIdentifier tableIdentifier) { PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getPassthroughResolvedPath( tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); if (resolvedEntities == null) { - throw new NoSuchTableException("Table does not exist: %s", tableIdentifier); + throw new NoSuchTableException("Generic table does not exist: %s", tableIdentifier); } List catalogPath = resolvedEntities.getRawParentPath(); @@ -187,6 +187,9 @@ public boolean dropGenericTable(TableIdentifier tableIdentifier) { public List listGenericTables(Namespace namespace) { PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); + if (resolvedEntities == null) { + throw new NoSuchNamespaceException("Namespace '%s' does not exist", namespace); + } List catalogPath = resolvedEntities.getRawFullPath(); List entities = 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 00469cc491..b71c7c46df 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 @@ -1365,7 +1365,7 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { } if (null == existingLocation) { - throw new NoSuchTableException("Table does not exist: %s", tableName()); + throw new NoSuchTableException("Iceberg table does not exist: %s", tableName()); } throw new CommitFailedException( @@ -1642,7 +1642,8 @@ private void renameTableLike( if (subType == PolarisEntitySubType.VIEW) { throw new NoSuchViewException("Cannot rename %s to %s. View does not exist", from, to); } else { - throw new NoSuchTableException("Cannot rename %s to %s. Table does not exist", from, to); + throw new NoSuchTableException( + "Cannot rename %s to %s. Iceberg table does not exist", from, to); } } List catalogPath = resolvedEntities.getRawParentPath(); From dab727969c0f55a0be0d17e29bed7a856f116dc9 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 10:19:43 -0700 Subject: [PATCH 06/23] revert iceberg messages to comply with oss tests --- .../service/it/test/PolarisApplicationIntegrationTest.java | 2 +- .../polaris/service/catalog/iceberg/IcebergCatalog.java | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java index 0be4312c04..738ac9709c 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java @@ -558,7 +558,7 @@ public void testIcebergDropTableInExternalCatalog() throws IOException { sessionCatalog.dropTable(sessionContext, tableIdentifier); assertThatThrownBy(() -> sessionCatalog.loadTable(sessionContext, tableIdentifier)) .isInstanceOf(NoSuchTableException.class) - .hasMessage("Table does not exist: db1.the_table"); + .hasMessage("Iceberg table does not exist: db1.the_table"); } } 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 b71c7c46df..00469cc491 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 @@ -1365,7 +1365,7 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { } if (null == existingLocation) { - throw new NoSuchTableException("Iceberg table does not exist: %s", tableName()); + throw new NoSuchTableException("Table does not exist: %s", tableName()); } throw new CommitFailedException( @@ -1642,8 +1642,7 @@ private void renameTableLike( if (subType == PolarisEntitySubType.VIEW) { throw new NoSuchViewException("Cannot rename %s to %s. View does not exist", from, to); } else { - throw new NoSuchTableException( - "Cannot rename %s to %s. Iceberg table does not exist", from, to); + throw new NoSuchTableException("Cannot rename %s to %s. Table does not exist", from, to); } } List catalogPath = resolvedEntities.getRawParentPath(); From 54a47a5458ff64b3b4187e03016497e21341fb4f Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 10:23:02 -0700 Subject: [PATCH 07/23] another revert --- .../service/it/test/PolarisApplicationIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java index 738ac9709c..0be4312c04 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java @@ -558,7 +558,7 @@ public void testIcebergDropTableInExternalCatalog() throws IOException { sessionCatalog.dropTable(sessionContext, tableIdentifier); assertThatThrownBy(() -> sessionCatalog.loadTable(sessionContext, tableIdentifier)) .isInstanceOf(NoSuchTableException.class) - .hasMessage("Iceberg table does not exist: db1.the_table"); + .hasMessage("Table does not exist: db1.the_table"); } } From 1ffbf078b500e1fcad784e7dbee25868fbdd524c Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 11:06:40 -0700 Subject: [PATCH 08/23] more iceberg catalog changes --- .../catalog/GenericTableCatalogTest.java | 47 +++++++++++++++++++ .../catalog/generic/GenericTableCatalog.java | 15 ++++-- .../catalog/iceberg/IcebergCatalog.java | 23 ++++++--- 3 files changed, 76 insertions(+), 9 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java index 62a5a300ff..e626bec14b 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java @@ -318,6 +318,36 @@ public void testCreateGenericTableDoesNotThrow() { .doesNotThrowAnyException(); } + @Test + public void testGenericTableAlreadyExists() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format1", Map.of()); + + Assertions.assertThatCode( + () -> genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format2", Map.of())) + .hasMessageContaining("already exists"); + + Assertions.assertThatCode( + () -> icebergCatalog.createTable(TableIdentifier.of("ns", "t1"), SCHEMA)) + .hasMessageContaining("already exists"); + } + + @Test + public void testIcebergTableAlreadyExists() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + icebergCatalog.createTable(TableIdentifier.of("ns", "t1"), SCHEMA); + + Assertions.assertThatCode( + () -> genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format2", Map.of())) + .hasMessageContaining("already exists"); + + Assertions.assertThatCode( + () -> icebergCatalog.createTable(TableIdentifier.of("ns", "t1"), SCHEMA)) + .hasMessageContaining("already exists"); + } + @Test public void testGenericTableRoundTrip() { Namespace namespace = Namespace.of("ns"); @@ -454,6 +484,23 @@ public void testListIcebergTables() { Assertions.assertThat(genericTableCatalog.listGenericTables(namespace)).isEmpty(); } + @Test + public void testListMixedTables() { + Namespace namespace = Namespace.of("ns"); + icebergCatalog.createNamespace(namespace); + + for (int i = 0; i < 10; i++) { + icebergCatalog.createTable(TableIdentifier.of("ns", "i" + i), SCHEMA); + } + + for (int i = 0; i < 10; i++) { + genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "g" + i), "format", Map.of()); + } + + Assertions.assertThat(genericTableCatalog.listGenericTables(namespace).size()).isEqualTo(10); + Assertions.assertThat(icebergCatalog.listTables(namespace).size()).isEqualTo(10); + } + @Test public void testDropNonExistentTable() { Namespace namespace = Namespace.of("ns"); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index d284659080..012b5fc734 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -88,6 +88,17 @@ public GenericTableCatalog( this.metaStoreManager = metaStoreManager; } + private PolarisResolvedPathWrapper getTablePath(TableIdentifier tableIdentifier) { + PolarisResolvedPathWrapper genericTableResult = resolvedEntityView.getPassthroughResolvedPath( + tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); + if (genericTableResult == null) { + return resolvedEntityView.getPassthroughResolvedPath( + tableIdentifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); + } else { + return genericTableResult; + } + } + public void createGenericTable( TableIdentifier tableIdentifier, String format, Map properties) { PolarisResolvedPathWrapper resolvedParent = @@ -101,9 +112,7 @@ public void createGenericTable( List catalogPath = resolvedParent.getRawFullPath(); - PolarisResolvedPathWrapper resolvedEntities = - resolvedEntityView.getPassthroughResolvedPath( - tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); + PolarisResolvedPathWrapper resolvedEntities = getTablePath(tableIdentifier); GenericTableEntity entity = GenericTableEntity.of( resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); 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 00469cc491..0c6c28afcf 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 @@ -1189,6 +1189,17 @@ private class BasePolarisTableOperations extends BaseMetastoreTableOperations { this.tableFileIO = defaultFileIO; } + protected PolarisResolvedPathWrapper getTablePath(TableIdentifier tableIdentifier) { + PolarisResolvedPathWrapper icebergTableResult = resolvedEntityView.getPassthroughResolvedPath( + tableIdentifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); + if (icebergTableResult == null) { + return resolvedEntityView.getPassthroughResolvedPath( + tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); + } else { + return icebergTableResult; + } + } + @Override public void doRefresh() { LOGGER.debug("doRefresh for tableIdentifier {}", tableIdentifier); @@ -1334,9 +1345,7 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { // concurrent // modification between our checking of unchanged metadataLocation here and actual // persistence-layer commit). - PolarisResolvedPathWrapper resolvedEntities = - resolvedEntityView.getPassthroughResolvedPath( - tableIdentifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.TABLE); + PolarisResolvedPathWrapper resolvedEntities = getTablePath(tableIdentifier); IcebergTableLikeEntity entity = IcebergTableLikeEntity.of( resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); @@ -1359,6 +1368,9 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { .setMetadataLocation(newLocation) .build(); } + if (entity.getType() == PolarisEntityType.GENERIC_TABLE) { + throw new AlreadyExistsException("Table already exists: %s", tableName()); + } if (!Objects.equal(existingLocation, oldLocation)) { if (null == base) { throw new AlreadyExistsException("Table already exists: %s", tableName()); @@ -1487,9 +1499,8 @@ public void doCommit(ViewMetadata base, ViewMetadata metadata) { identifier, identifier.namespace()); } - PolarisResolvedPathWrapper resolvedTable = - resolvedEntityView.getPassthroughResolvedPath( - identifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.TABLE); + PolarisResolvedPathWrapper resolvedTable = resolvedEntityView.getPassthroughResolvedPath( + identifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.TABLE); if (resolvedTable != null) { throw new AlreadyExistsException("Table with same name already exists: %s", identifier); } From 113c9419f750fd753ad24befcc27edb3874e720f Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 11:08:35 -0700 Subject: [PATCH 09/23] autolint --- .../quarkus/catalog/GenericTableCatalogTest.java | 8 ++++++-- .../service/catalog/generic/GenericTableCatalog.java | 5 +++-- .../service/catalog/iceberg/IcebergCatalog.java | 12 ++++++++---- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java index e626bec14b..9fc59cce24 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java @@ -325,7 +325,9 @@ public void testGenericTableAlreadyExists() { genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format1", Map.of()); Assertions.assertThatCode( - () -> genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format2", Map.of())) + () -> + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", "t1"), "format2", Map.of())) .hasMessageContaining("already exists"); Assertions.assertThatCode( @@ -340,7 +342,9 @@ public void testIcebergTableAlreadyExists() { icebergCatalog.createTable(TableIdentifier.of("ns", "t1"), SCHEMA); Assertions.assertThatCode( - () -> genericTableCatalog.createGenericTable(TableIdentifier.of("ns", "t1"), "format2", Map.of())) + () -> + genericTableCatalog.createGenericTable( + TableIdentifier.of("ns", "t1"), "format2", Map.of())) .hasMessageContaining("already exists"); Assertions.assertThatCode( diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index 012b5fc734..322b7cb0db 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -89,8 +89,9 @@ public GenericTableCatalog( } private PolarisResolvedPathWrapper getTablePath(TableIdentifier tableIdentifier) { - PolarisResolvedPathWrapper genericTableResult = resolvedEntityView.getPassthroughResolvedPath( - tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); + PolarisResolvedPathWrapper genericTableResult = + resolvedEntityView.getPassthroughResolvedPath( + tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); if (genericTableResult == null) { return resolvedEntityView.getPassthroughResolvedPath( tableIdentifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); 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 0c6c28afcf..54434a0754 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 @@ -1190,8 +1190,11 @@ private class BasePolarisTableOperations extends BaseMetastoreTableOperations { } protected PolarisResolvedPathWrapper getTablePath(TableIdentifier tableIdentifier) { - PolarisResolvedPathWrapper icebergTableResult = resolvedEntityView.getPassthroughResolvedPath( - tableIdentifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE); + PolarisResolvedPathWrapper icebergTableResult = + resolvedEntityView.getPassthroughResolvedPath( + tableIdentifier, + PolarisEntityType.ICEBERG_TABLE_LIKE, + PolarisEntitySubType.ANY_SUBTYPE); if (icebergTableResult == null) { return resolvedEntityView.getPassthroughResolvedPath( tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); @@ -1499,8 +1502,9 @@ public void doCommit(ViewMetadata base, ViewMetadata metadata) { identifier, identifier.namespace()); } - PolarisResolvedPathWrapper resolvedTable = resolvedEntityView.getPassthroughResolvedPath( - identifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.TABLE); + PolarisResolvedPathWrapper resolvedTable = + resolvedEntityView.getPassthroughResolvedPath( + identifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.TABLE); if (resolvedTable != null) { throw new AlreadyExistsException("Table with same name already exists: %s", identifier); } From b672986849ea636ba3faf50e17403aec57aeb429 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 11:54:55 -0700 Subject: [PATCH 10/23] dependency issues --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 6 + .../core/config/FeatureConfiguration.java | 8 + .../core/persistence/BasePersistence.java | 3 + .../pagination/OffsetPageToken.java | 113 ++++++++++ .../persistence/pagination/PageToken.java | 179 +++++++++++++++ .../persistence/pagination/PolarisPage.java | 42 ++++ .../pagination/ReadEverythingPageToken.java | 95 ++++++++ service/common/build.gradle.kts | 3 + .../catalog/iceberg/IcebergCatalog.java | 2 + .../pagination/EntityIdPageToken.java | 115 ++++++++++ .../ListNamespacesResponseWithPageToken.java | 72 ++++++ .../persistence/pagination/PageTokenTest.java | 207 ++++++++++++++++++ 12 files changed, 845 insertions(+) create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/OffsetPageToken.java create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageToken.java create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PolarisPage.java create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/ReadEverythingPageToken.java create mode 100644 service/common/src/main/java/org/apache/polaris/service/persistence/pagination/EntityIdPageToken.java create mode 100644 service/common/src/main/java/org/apache/polaris/service/types/ListNamespacesResponseWithPageToken.java create mode 100644 service/common/src/test/java/org/apache/polaris/service/persistence/pagination/PageTokenTest.java diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 7005d5cfea..e48ee1060a 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -52,6 +52,7 @@ import org.apache.polaris.core.persistence.BaseMetaStoreManager; import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; +import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.transactional.AbstractTransactionalPersistence; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -671,4 +672,9 @@ public void rollback() { session.getTransaction().rollback(); } } + + @Override + public PageToken.PageTokenBuilder pageTokenBuilder() { + return EntityIdPageToken.builder(); + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java index 087d0525ee..75ebbeab29 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java @@ -183,4 +183,12 @@ protected FeatureConfiguration( "How many times to retry refreshing metadata when the previous error was retryable") .defaultValue(2) .buildFeatureConfiguration(); + + public static final PolarisConfiguration LIST_PAGINATION_ENABLED = + PolarisConfiguration.builder() + .key("LIST_PAGINATION_ENABLED") + .catalogConfig("list-pagination.enabled") + .description("If set to true, pagination for APIs like listTables is enabled") + .defaultValue(false) + .buildFeatureConfiguration(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index 88e5143cf8..b58c1b909f 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -31,6 +31,7 @@ import org.apache.polaris.core.entity.PolarisEntityId; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PolarisGrantRecord; +import org.apache.polaris.core.persistence.pagination.PageToken; /** * Interface to the Polaris persistence backend, with which to persist and retrieve all the data @@ -403,4 +404,6 @@ boolean hasChildren( default BasePersistence detach() { return this; } + + PageToken.PageTokenBuilder pageTokenBuilder(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/OffsetPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/OffsetPageToken.java new file mode 100644 index 0000000000..ea0e8be1c7 --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/OffsetPageToken.java @@ -0,0 +1,113 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.persistence.pagination; + +import java.util.List; + +/** + * A simple {@link PageToken} implementation that tracks the number of records returned. Entities + * are meant to be filtered during listing such that when a token with offset N is supplied, the + * first N records are omitted from the results. + */ +public class OffsetPageToken extends PageToken { + + /** + * The offset of the token. If this is `5` for example, the first 5 entities returned by a list + * operation that uses this token will be skipped. + */ + public final int offset; + + /** The offset to use to start with. */ + private static final int BASE_OFFSET = 0; + + private OffsetPageToken(int offset, int pageSize) { + this.offset = offset; + this.pageSize = pageSize; + validate(); + } + + @Override + protected void validate() { + if (offset < 0) { + throw new IllegalArgumentException("Offset must be greater than zero"); + } + super.validate(); + } + + /** Get a new `EntityIdPageTokenBuilder` instance */ + public static PageTokenBuilder builder() { + return new OffsetPageTokenBuilder(); + } + + @Override + protected PageTokenBuilder getBuilder() { + return OffsetPageToken.builder(); + } + + @Override + protected List getComponents() { + return List.of(String.valueOf(this.offset), String.valueOf(this.pageSize)); + } + + /** A {@link PageTokenBuilder} implementation for {@link OffsetPageToken} */ + public static class OffsetPageTokenBuilder extends PageTokenBuilder { + + private OffsetPageTokenBuilder() {} + + @Override + public String tokenPrefix() { + return "polaris-offset"; + } + + @Override + public int expectedComponents() { + // offset + limit + return 2; + } + + @Override + protected OffsetPageToken fromStringComponents(List components) { + return new OffsetPageToken( + Integer.parseInt(components.get(0)), Integer.parseInt(components.get(1))); + } + + @Override + protected OffsetPageToken fromLimitImpl(int limit) { + return new OffsetPageToken(BASE_OFFSET, limit); + } + } + + @Override + public PageToken updated(List newData) { + if (newData == null || newData.size() < this.pageSize) { + return PageToken.DONE; + } else { + return new OffsetPageToken(this.offset + newData.size(), pageSize); + } + } + + @Override + public OffsetPageToken withPageSize(Integer pageSize) { + if (pageSize == null) { + return new OffsetPageToken(BASE_OFFSET, this.pageSize); + } else { + return new OffsetPageToken(this.offset, pageSize); + } + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageToken.java new file mode 100644 index 0000000000..a9ba699877 --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageToken.java @@ -0,0 +1,179 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.persistence.pagination; + +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Base64; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * Represents a page token that can be used by operations like `listTables`. Clients that specify a + * `pageSize` (or a `pageToken`) may receive a `next-page-token` in the response, the content of + * which is a serialized PageToken. + * + *

By providing that in the next query's `pageToken`, the client can resume listing where they + * left off. If the client provides a `pageToken` or `pageSize` but `next-page-token` is null in the + * response, that means there is no more data to read. + */ +public abstract class PageToken { + + public int pageSize; + + public static final PageToken DONE = null; + public static final int DEFAULT_PAGE_SIZE = 1000; + + protected void validate() { + if (pageSize <= 0) { + throw new IllegalArgumentException("Page size must be greater than zero"); + } + } + + /** + * Get a new PageTokenBuilder from a PageToken. The PageTokenBuilder type should match the + * PageToken type. Implementations may also provide a static `builder` method to obtain the same + * PageTokenBuilder. + */ + protected abstract PageTokenBuilder getBuilder(); + + /** Allows `PageToken` implementations to implement methods like `fromLimit` */ + public abstract static class PageTokenBuilder { + + /** + * A prefix that tokens are expected to start with, ideally unique across `PageTokenBuilder` + * implementations. + */ + public abstract String tokenPrefix(); + + /** + * The number of expected components in a token. This should match the number of components + * returned by getComponents and shouldn't account for the prefix or the checksum. + */ + public abstract int expectedComponents(); + + /** Deserialize a string into a {@link PageToken} */ + public final PageToken fromString(String tokenString) { + if (tokenString == null) { + throw new IllegalArgumentException("Cannot build page token from null string"); + } else if (tokenString.isEmpty()) { + if (this instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + return ReadEverythingPageToken.get(); + } else { + return fromLimit(DEFAULT_PAGE_SIZE); + } + } else { + try { + String decoded = + new String(Base64.getDecoder().decode(tokenString), StandardCharsets.UTF_8); + String[] parts = decoded.split(":"); + + // +2 to account for the prefix and checksum. + if (parts.length != expectedComponents() + 2 || !parts[0].equals(tokenPrefix())) { + throw new IllegalArgumentException("Invalid token format in token: " + tokenString); + } + + // Cut off prefix and checksum + T result = fromStringComponents(Arrays.asList(parts).subList(1, parts.length - 1)); + result.validate(); + return result; + } catch (Exception e) { + throw new IllegalArgumentException("Failed to decode page token: " + tokenString, e); + } + } + } + + /** Construct a {@link PageToken} from a plain limit */ + public final PageToken fromLimit(Integer limit) { + if (limit == null) { + return ReadEverythingPageToken.get(); + } else { + return fromLimitImpl(limit); + } + } + + /** Construct a {@link PageToken} from a plain limit */ + protected abstract T fromLimitImpl(int limit); + + /** + * {@link PageTokenBuilder} implementations should implement this to build a {@link PageToken} + * from components in a string token. These components should be the same ones returned by + * {@link #getComponents()} and won't include the token prefix or the checksum. + */ + protected abstract T fromStringComponents(List components); + } + + /** Convert this into components that the serialized token string will be built from. */ + protected abstract List getComponents(); + + /** + * Builds a new page token to reflect new data that's been read. If the amount of data read is + * less than the pageSize, this will return {@link PageToken#DONE}(null) + */ + protected abstract PageToken updated(List newData); + + /** + * Builds a {@link PolarisPage} from a {@link List}. The {@link PageToken} attached to the + * new {@link PolarisPage} is the same as the result of calling {@link #updated(List)} on this + * {@link PageToken}. + */ + public final PolarisPage buildNextPage(List data) { + return new PolarisPage(updated(data), data); + } + + /** + * Return a new {@link PageToken} with an updated page size. If the pageSize provided is null, the + * existing page size will be preserved. + */ + public abstract PageToken withPageSize(Integer pageSize); + + /** Serialize a {@link PageToken} into a string */ + @Override + public final String toString() { + List components = getComponents(); + String prefix = getBuilder().tokenPrefix(); + String componentString = String.join(":", components); + String checksum = String.valueOf(componentString.hashCode()); + List allElements = + Stream.of(prefix, componentString, checksum) + .filter(s -> !s.isEmpty()) + .collect(Collectors.toList()); + String rawString = String.join(":", allElements); + return Base64.getEncoder().encodeToString(rawString.getBytes(StandardCharsets.UTF_8)); + } + + @Override + public final boolean equals(Object o) { + if (o instanceof PageToken) { + return this.toString().equals(o.toString()); + } else { + return false; + } + } + + @Override + public final int hashCode() { + if (toString() == null) { + return 0; + } else { + return toString().hashCode(); + } + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PolarisPage.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PolarisPage.java new file mode 100644 index 0000000000..085f13ff18 --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PolarisPage.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.persistence.pagination; + +import java.util.List; + +/** + * A wrapper for a {@link List} of data and a {@link PageToken} that can be used to continue the + * listing operation that generated that data. + */ +public class PolarisPage { + public final PageToken pageToken; + public final List data; + + public PolarisPage(PageToken pageToken, List data) { + this.pageToken = pageToken; + this.data = data; + } + + /** + * Used to wrap a {@link List} of data into a {@link PolarisPage} when there is no more data + */ + public static PolarisPage fromData(List data) { + return new PolarisPage<>(PageToken.DONE, data); + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/ReadEverythingPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/ReadEverythingPageToken.java new file mode 100644 index 0000000000..5d6d296258 --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/ReadEverythingPageToken.java @@ -0,0 +1,95 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.persistence.pagination; + +import java.util.List; + +/** + * A {@link PageToken} implementation for readers who want to read everything. The behavior when + * using this token should be the same as when reading without a token. + */ +public class ReadEverythingPageToken extends PageToken { + + private ReadEverythingPageToken() { + this.pageSize = Integer.MAX_VALUE; + validate(); + } + + /** Get a {@link ReadEverythingPageToken} */ + public static ReadEverythingPageToken get() { + return new ReadEverythingPageToken(); + } + + public static PageTokenBuilder builder() { + return new ReadEverythingPageTokenBuilder(); + } + + @Override + protected PageTokenBuilder getBuilder() { + return builder(); + } + + /** A {@link PageTokenBuilder} implementation for {@link ReadEverythingPageToken} */ + public static class ReadEverythingPageTokenBuilder + extends PageTokenBuilder { + + private ReadEverythingPageTokenBuilder() {} + + @Override + public String tokenPrefix() { + return "polaris-read-everything"; + } + + @Override + public int expectedComponents() { + return 0; + } + + @Override + protected ReadEverythingPageToken fromStringComponents(List components) { + return ReadEverythingPageToken.get(); + } + + @Override + protected ReadEverythingPageToken fromLimitImpl(int limit) { + throw new UnsupportedOperationException(); + } + } + + @Override + protected List getComponents() { + return List.of(); + } + + /** Any time {@link ReadEverythingPageToken} is updated, everything has been read */ + @Override + public PageToken updated(List newData) { + return PageToken.DONE; + } + + /** {@link ReadEverythingPageToken} does not support page size */ + @Override + public PageToken withPageSize(Integer pageSize) { + if (pageSize == null) { + return ReadEverythingPageToken.get(); + } else { + throw new UnsupportedOperationException(); + } + } +} diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts index a98ef50a9f..4f1fb9077e 100644 --- a/service/common/build.gradle.kts +++ b/service/common/build.gradle.kts @@ -28,6 +28,7 @@ dependencies { implementation(project(":polaris-api-management-model")) implementation(project(":polaris-api-management-service")) implementation(project(":polaris-api-iceberg-service")) + implementation(project(":polaris-jpa-model")) implementation(platform(libs.iceberg.bom)) implementation("org.apache.iceberg:iceberg-api") @@ -101,6 +102,8 @@ dependencies { testFixturesImplementation(project(":polaris-api-management-model")) testFixturesImplementation(project(":polaris-api-management-service")) testFixturesImplementation(project(":polaris-api-iceberg-service")) + testFixturesImplementation(project(":polaris-jpa-model")) + testFixturesImplementation(libs.jakarta.enterprise.cdi.api) testFixturesImplementation(libs.jakarta.annotation.api) 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 54434a0754..9f18f7e567 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 @@ -111,6 +111,7 @@ import org.apache.polaris.service.catalog.SupportsNotifications; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.catalog.io.FileIOUtil; +import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.service.task.TaskExecutor; import org.apache.polaris.service.types.NotificationRequest; import org.apache.polaris.service.types.NotificationType; @@ -169,6 +170,7 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog private Map tableDefaultProperties; private FileIOFactory fileIOFactory; private PolarisMetaStoreManager metaStoreManager; + private final PageToken.PageTokenBuilder pageTokenBuilder; /** * @param entityManager provides handle to underlying PolarisMetaStoreManager with which to diff --git a/service/common/src/main/java/org/apache/polaris/service/persistence/pagination/EntityIdPageToken.java b/service/common/src/main/java/org/apache/polaris/service/persistence/pagination/EntityIdPageToken.java new file mode 100644 index 0000000000..e280665392 --- /dev/null +++ b/service/common/src/main/java/org/apache/polaris/service/persistence/pagination/EntityIdPageToken.java @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.service.persistence.pagination; + +import java.util.List; +import org.apache.polaris.core.entity.PolarisBaseEntity; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.jpa.models.ModelEntity; + +/** + * A {@link PageToken} implementation that tracks the greatest ID from either {@link + * PolarisBaseEntity} or {@link ModelEntity} objects supplied in updates. Entities are meant to be + * filtered during listing such that only entities with and ID greater than the ID of the token are + * returned. + */ +public class EntityIdPageToken extends PageToken { + public long id; + + private EntityIdPageToken(long id, int pageSize) { + this.id = id; + this.pageSize = pageSize; + validate(); + } + + /** The minimum ID that could be attached to an entity */ + private static final long MINIMUM_ID = 0; + + /** The entity ID to use to start with. */ + private static final long BASE_ID = MINIMUM_ID - 1; + + @Override + protected List getComponents() { + return List.of(String.valueOf(id), String.valueOf(pageSize)); + } + + /** Get a new `EntityIdPageTokenBuilder` instance */ + public static PageTokenBuilder builder() { + return new EntityIdPageToken.EntityIdPageTokenBuilder(); + } + + @Override + protected PageTokenBuilder getBuilder() { + return EntityIdPageToken.builder(); + } + + /** A {@link PageTokenBuilder} implementation for {@link EntityIdPageToken} */ + public static class EntityIdPageTokenBuilder extends PageTokenBuilder { + + @Override + public String tokenPrefix() { + return "polaris-entity-id"; + } + + @Override + public int expectedComponents() { + // id, pageSize + return 2; + } + + @Override + protected EntityIdPageToken fromStringComponents(List components) { + return new EntityIdPageToken( + Integer.parseInt(components.get(0)), Integer.parseInt(components.get(1))); + } + + @Override + protected EntityIdPageToken fromLimitImpl(int limit) { + return new EntityIdPageToken(BASE_ID, limit); + } + } + + @Override + public PageToken updated(List newData) { + if (newData == null || newData.size() < this.pageSize) { + return DONE; + } else { + var head = newData.get(0); + if (head instanceof ModelEntity) { + return new EntityIdPageToken( + ((ModelEntity) newData.get(newData.size() - 1)).getId(), this.pageSize); + } else if (head instanceof PolarisBaseEntity) { + return new EntityIdPageToken( + ((PolarisBaseEntity) newData.get(newData.size() - 1)).getId(), this.pageSize); + } else { + throw new IllegalArgumentException( + "Cannot build a page token from: " + newData.get(0).getClass().getSimpleName()); + } + } + } + + @Override + public PageToken withPageSize(Integer pageSize) { + if (pageSize == null) { + return new EntityIdPageToken(BASE_ID, this.pageSize); + } else { + return new EntityIdPageToken(this.id, pageSize); + } + } +} diff --git a/service/common/src/main/java/org/apache/polaris/service/types/ListNamespacesResponseWithPageToken.java b/service/common/src/main/java/org/apache/polaris/service/types/ListNamespacesResponseWithPageToken.java new file mode 100644 index 0000000000..cbfaed2998 --- /dev/null +++ b/service/common/src/main/java/org/apache/polaris/service/types/ListNamespacesResponseWithPageToken.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.service.types; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.MoreObjects; +import com.google.common.base.Preconditions; +import java.util.List; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.rest.responses.ListNamespacesResponse; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; + +/** + * Used in lieu of {@link ListNamespacesResponse} when there may be a {@link PageToken} associated + * with the response. Callers can use this {@link PageToken} to continue the listing operation and + * obtain more results. + */ +public class ListNamespacesResponseWithPageToken extends ListNamespacesResponse { + private final PageToken pageToken; + + private final List namespaces; + + public ListNamespacesResponseWithPageToken(PageToken pageToken, List namespaces) { + this.pageToken = pageToken; + this.namespaces = namespaces; + Preconditions.checkArgument(this.namespaces != null, "Invalid namespace: null"); + } + + public static ListNamespacesResponseWithPageToken fromPolarisPage( + PolarisPage polarisPage) { + return new ListNamespacesResponseWithPageToken(polarisPage.pageToken, polarisPage.data); + } + + @JsonProperty("next-page-token") + public String getPageToken() { + if (pageToken == null) { + return null; + } else { + return pageToken.toString(); + } + } + + @Override + public List namespaces() { + return this.namespaces != null ? this.namespaces : List.of(); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("namespaces", this.namespaces) + .add("pageToken", this.pageToken.toString()) + .toString(); + } +} diff --git a/service/common/src/test/java/org/apache/polaris/service/persistence/pagination/PageTokenTest.java b/service/common/src/test/java/org/apache/polaris/service/persistence/pagination/PageTokenTest.java new file mode 100644 index 0000000000..93af6dbe03 --- /dev/null +++ b/service/common/src/test/java/org/apache/polaris/service/persistence/pagination/PageTokenTest.java @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.service.persistence.pagination; + +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.apache.polaris.core.persistence.pagination.OffsetPageToken; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; +import org.apache.polaris.jpa.models.ModelEntity; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class PageTokenTest { + private static final Logger LOGGER = LoggerFactory.getLogger(PageTokenTest.class); + + static Stream> getPageTokenBuilders() { + return Stream.of( + OffsetPageToken.builder(), EntityIdPageToken.builder(), ReadEverythingPageToken.builder()); + } + + @Test + void testDoneToken() { + Assertions.assertThat(PageToken.DONE).isNull(); + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testRoundTrips(PageToken.PageTokenBuilder builder) { + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + // Skip ReadEverythingPageToken + return; + } + + for (int limit : List.of(1, 10, 100, Integer.MAX_VALUE)) { + PageToken token = builder.fromLimit(limit); + Assertions.assertThat(token.pageSize).isEqualTo(limit); + Assertions.assertThat(builder.fromString(token.toString())).isEqualTo(token); + } + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testInvalidLimits(PageToken.PageTokenBuilder builder) { + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + // Skip ReadEverythingPageToken + return; + } + + for (int limit : List.of(-1, 0)) { + Assertions.assertThatThrownBy(() -> builder.fromLimit(limit)) + .isInstanceOf(IllegalArgumentException.class); + } + + Assertions.assertThat(builder.fromLimit(null)).isInstanceOf(ReadEverythingPageToken.class); + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testStartingTokens(PageToken.PageTokenBuilder builder) { + Assertions.assertThat(builder.fromString("")).isNotNull(); + if (!(builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder)) { + Assertions.assertThat(builder.fromString("")).isNotEqualTo(ReadEverythingPageToken.get()); + } + + Assertions.assertThatThrownBy(() -> builder.fromString(null)) + .isInstanceOf(IllegalArgumentException.class); + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testPageBuilding(PageToken.PageTokenBuilder builder) { + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + // Skip ReadEverythingPageToken + return; + } + + List data = + List.of(ModelEntity.builder().id(1).build(), ModelEntity.builder().id(2).build()); + + PageToken token = builder.fromLimit(1000); + Assertions.assertThat(token.buildNextPage(data).data).isEqualTo(data); + Assertions.assertThat(token.buildNextPage(data).pageToken).isNull(); + } + + @Test + void testUniquePrefixes() { + Stream> builders = getPageTokenBuilders(); + List prefixes = + builders.map(PageToken.PageTokenBuilder::tokenPrefix).collect(Collectors.toList()); + Assertions.assertThat(prefixes.size()).isEqualTo(prefixes.stream().distinct().count()); + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testCrossTokenParsing(PageToken.PageTokenBuilder builder) { + var otherBuilders = getPageTokenBuilders().collect(Collectors.toList()); + for (var otherBuilder : otherBuilders) { + LOGGER.info( + "Testing {} being parsed by {}", + builder.getClass().getSimpleName(), + otherBuilder.getClass().getSimpleName()); + + final PageToken token; + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + token = ReadEverythingPageToken.get(); + } else { + token = builder.fromLimit(1234); + } + if (otherBuilder.getClass().equals(builder.getClass())) { + Assertions.assertThat(otherBuilder.fromString(token.toString())).isEqualTo(token); + } else { + Assertions.assertThatThrownBy(() -> otherBuilder.fromString(token.toString())) + .isInstanceOf(IllegalArgumentException.class); + } + } + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testDefaultTokens(PageToken.PageTokenBuilder builder) { + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + // Skip ReadEverythingPageToken + return; + } + + PageToken token = builder.fromString(""); + Assertions.assertThat(token.toString()).isNotNull(); + Assertions.assertThat(token.pageSize).isEqualTo(PageToken.DEFAULT_PAGE_SIZE); + } + + @Test + void testReadEverythingPageToken() { + PageToken token = ReadEverythingPageToken.get(); + + Assertions.assertThat(token.toString()).isNotNull(); + Assertions.assertThat(token.updated(List.of("anything"))).isEqualTo(PageToken.DONE); + Assertions.assertThat(token.pageSize).isEqualTo(Integer.MAX_VALUE); + } + + @Test + void testOffsetPageToken() { + OffsetPageToken token = (OffsetPageToken) OffsetPageToken.builder().fromLimit(2); + + Assertions.assertThat(token).isInstanceOf(OffsetPageToken.class); + Assertions.assertThat(token.offset).isEqualTo(0); + + List data = List.of("some", "data"); + var page = token.buildNextPage(data); + Assertions.assertThat(page.pageToken).isNotNull(); + Assertions.assertThat(page.pageToken).isInstanceOf(OffsetPageToken.class); + Assertions.assertThat(page.pageToken.pageSize).isEqualTo(2); + Assertions.assertThat(((OffsetPageToken) page.pageToken).offset).isEqualTo(2); + Assertions.assertThat(page.data).isEqualTo(data); + + Assertions.assertThat(OffsetPageToken.builder().fromString(page.pageToken.toString())) + .isEqualTo(page.pageToken); + } + + @Test + void testEntityIdPageToken() { + EntityIdPageToken token = (EntityIdPageToken) EntityIdPageToken.builder().fromLimit(2); + + Assertions.assertThat(token).isInstanceOf(EntityIdPageToken.class); + Assertions.assertThat(token.id).isEqualTo(-1L); + + List badData = List.of("some", "data"); + Assertions.assertThatThrownBy(() -> token.buildNextPage(badData)) + .isInstanceOf(IllegalArgumentException.class); + + List data = + List.of(ModelEntity.builder().id(101).build(), ModelEntity.builder().id(102).build()); + var page = token.buildNextPage(data); + + Assertions.assertThat(page.pageToken).isNotNull(); + Assertions.assertThat(page.pageToken).isInstanceOf(EntityIdPageToken.class); + Assertions.assertThat(page.pageToken.pageSize).isEqualTo(2); + Assertions.assertThat(((EntityIdPageToken) page.pageToken).id).isEqualTo(102); + Assertions.assertThat(page.data).isEqualTo(data); + + Assertions.assertThat(EntityIdPageToken.builder().fromString(page.pageToken.toString())) + .isEqualTo(page.pageToken); + } +} From 4a9ed693c038b1460b14fe50fdeffceff38ef115 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 13:10:27 -0700 Subject: [PATCH 11/23] more wiring --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 3 +- .../jpa/models}/EntityIdPageToken.java | 3 +- .../core/persistence/BasePersistence.java | 1 + .../TreeMapTransactionalPersistenceImpl.java | 7 + service/common/build.gradle.kts | 1 - .../catalog/iceberg/IcebergCatalog.java | 3 +- .../iceberg/IcebergCatalogAdapter.java | 27 +- .../iceberg/IcebergCatalogHandlerWrapper.java | 43 +++ .../ListTablesResponseWithPageToken.java | 73 +++++ .../persistence/pagination/PageTokenTest.java | 310 +++++++++--------- 10 files changed, 308 insertions(+), 163 deletions(-) rename {service/common/src/main/java/org/apache/polaris/service/persistence/pagination => extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models}/EntityIdPageToken.java (97%) create mode 100644 service/common/src/main/java/org/apache/polaris/service/types/ListTablesResponseWithPageToken.java diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index e48ee1060a..7a377fde24 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -57,6 +57,7 @@ import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; +import org.apache.polaris.jpa.models.EntityIdPageToken; import org.apache.polaris.jpa.models.ModelEntity; import org.apache.polaris.jpa.models.ModelEntityActive; import org.apache.polaris.jpa.models.ModelEntityChangeTracking; @@ -674,7 +675,7 @@ public void rollback() { } @Override - public PageToken.PageTokenBuilder pageTokenBuilder() { + public @Nonnull PageToken.PageTokenBuilder pageTokenBuilder() { return EntityIdPageToken.builder(); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/persistence/pagination/EntityIdPageToken.java b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/EntityIdPageToken.java similarity index 97% rename from service/common/src/main/java/org/apache/polaris/service/persistence/pagination/EntityIdPageToken.java rename to extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/EntityIdPageToken.java index e280665392..1573082977 100644 --- a/service/common/src/main/java/org/apache/polaris/service/persistence/pagination/EntityIdPageToken.java +++ b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/EntityIdPageToken.java @@ -16,12 +16,11 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.polaris.service.persistence.pagination; +package org.apache.polaris.jpa.models; import java.util.List; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.persistence.pagination.PageToken; -import org.apache.polaris.jpa.models.ModelEntity; /** * A {@link PageToken} implementation that tracks the greatest ID from either {@link diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index b58c1b909f..0f5be70805 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -405,5 +405,6 @@ default BasePersistence detach() { return this; } + @Nonnull PageToken.PageTokenBuilder pageTokenBuilder(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 8d2e59c963..7b849514b9 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -38,6 +38,8 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.BaseMetaStoreManager; import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; +import org.apache.polaris.core.persistence.pagination.OffsetPageToken; +import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; @@ -551,4 +553,9 @@ PolarisStorageIntegration loadPolarisStorageIntegrationInCurrentTxn( public void rollback() { this.store.rollback(); } + + @Override + public @Nonnull PageToken.PageTokenBuilder pageTokenBuilder() { + return OffsetPageToken.builder(); + } } diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts index 4f1fb9077e..d12539805e 100644 --- a/service/common/build.gradle.kts +++ b/service/common/build.gradle.kts @@ -104,7 +104,6 @@ dependencies { testFixturesImplementation(project(":polaris-api-iceberg-service")) testFixturesImplementation(project(":polaris-jpa-model")) - testFixturesImplementation(libs.jakarta.enterprise.cdi.api) testFixturesImplementation(libs.jakarta.annotation.api) testFixturesImplementation(libs.jakarta.ws.rs.api) 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 9f18f7e567..9af0d977a6 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 @@ -98,6 +98,7 @@ import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult; +import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.apache.polaris.core.persistence.resolver.ResolverPath; @@ -111,7 +112,6 @@ import org.apache.polaris.service.catalog.SupportsNotifications; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.catalog.io.FileIOUtil; -import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.service.task.TaskExecutor; import org.apache.polaris.service.types.NotificationRequest; import org.apache.polaris.service.types.NotificationType; @@ -199,6 +199,7 @@ public IcebergCatalog( this.catalogName = catalogEntity.getName(); this.fileIOFactory = fileIOFactory; this.metaStoreManager = metaStoreManager; + this.pageTokenBuilder = callContext.getPolarisCallContext().getMetaStore().pageTokenBuilder(); } @Override diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index d88ea3a7e4..e7c0f13114 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import jakarta.annotation.Nullable; import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; import jakarta.ws.rs.core.Response; @@ -63,6 +64,7 @@ import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.ResolvedPolarisEntity; +import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.resolver.Resolver; import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.service.catalog.AccessDelegationMode; @@ -186,6 +188,22 @@ private IcebergCatalogHandlerWrapper newHandlerWrapper( polarisAuthorizer); } + /** Build a {@link PageToken} from a string and page size. */ + private PageToken buildPageToken( + @Nullable String tokenString, + @Nullable Integer pageSize) { + if (tokenString != null) { + return callContext + .getPolarisCallContext() + .getMetaStore() + .pageTokenBuilder() + .fromString(tokenString) + .withPageSize(pageSize); + } else { + return callContext.getPolarisCallContext().getMetaStore().pageTokenBuilder().fromLimit(pageSize); + } + } + @Override public Response createNamespace( String prefix, @@ -208,11 +226,12 @@ public Response listNamespaces( SecurityContext securityContext) { Optional namespaceOptional = Optional.ofNullable(parent).map(IcebergCatalogAdapter::decodeNamespace); + PageToken token = buildPageToken(pageToken, pageSize); return withCatalog( securityContext, prefix, catalog -> - Response.ok(catalog.listNamespaces(namespaceOptional.orElse(Namespace.of()))).build()); + Response.ok(catalog.listNamespaces(namespaceOptional.orElse(Namespace.of()), token)).build()); } @Override @@ -320,8 +339,9 @@ public Response listTables( RealmContext realmContext, SecurityContext securityContext) { Namespace ns = decodeNamespace(namespace); + PageToken token = buildPageToken(pageToken, pageSize); return withCatalog( - securityContext, prefix, catalog -> Response.ok(catalog.listTables(ns)).build()); + securityContext, prefix, catalog -> Response.ok(catalog.listTables(ns, token)).build()); } @Override @@ -467,8 +487,9 @@ public Response listViews( RealmContext realmContext, SecurityContext securityContext) { Namespace ns = decodeNamespace(namespace); + PageToken token = buildPageToken(pageToken, pageSize); return withCatalog( - securityContext, prefix, catalog -> Response.ok(catalog.listViews(ns)).build()); + securityContext, prefix, catalog -> Response.ok(catalog.listViews(ns, token)).build()); } @Override diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java index 543ba907c6..734007daf0 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java @@ -86,12 +86,16 @@ import org.apache.polaris.core.persistence.TransactionWorkspaceMetaStoreManager; import org.apache.polaris.core.persistence.dao.entity.EntitiesResult; import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.SupportsNotifications; import org.apache.polaris.service.context.CallContextCatalogFactory; +import org.apache.polaris.service.types.ListNamespacesResponseWithPageToken; +import org.apache.polaris.service.types.ListTablesResponseWithPageToken; import org.apache.polaris.service.types.NotificationRequest; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -458,6 +462,20 @@ private void authorizeRenameTableLikeOperationOrThrow( initializeCatalog(); } + + public ListNamespacesResponseWithPageToken listNamespaces(Namespace parent, PageToken pageToken) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES; + authorizeBasicNamespaceOperationOrThrow(op, parent); + + if (baseCatalog instanceof IcebergCatalog polarisCatalog) { + return ListNamespacesResponseWithPageToken.fromPolarisPage(polarisCatalog.listNamespaces(parent, pageToken)); + } else { + return ListNamespacesResponseWithPageToken.fromPolarisPage( + PolarisPage.fromData( + CatalogHandlers.listNamespaces(namespaceCatalog, parent).namespaces())); + } + } + public ListNamespacesResponse listNamespaces(Namespace parent) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES; authorizeBasicNamespaceOperationOrThrow(op, parent); @@ -541,6 +559,18 @@ public UpdateNamespacePropertiesResponse updateNamespaceProperties( return CatalogHandlers.updateNamespaceProperties(namespaceCatalog, namespace, request); } + public ListTablesResponseWithPageToken listTables(Namespace namespace, PageToken pageToken) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_TABLES; + authorizeBasicNamespaceOperationOrThrow(op, namespace); + + if (baseCatalog instanceof IcebergCatalog polarisCatalog) { + return ListTablesResponseWithPageToken.fromPolarisPage(polarisCatalog.listTables(namespace, pageToken)); + } else { + return ListTablesResponseWithPageToken.fromPolarisPage( + PolarisPage.fromData(CatalogHandlers.listTables(baseCatalog, namespace).identifiers())); + } + } + public ListTablesResponse listTables(Namespace namespace) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_TABLES; authorizeBasicNamespaceOperationOrThrow(op, namespace); @@ -1061,6 +1091,19 @@ public void commitTransaction(CommitTransactionRequest commitTransactionRequest) } } + public ListTablesResponseWithPageToken listViews(Namespace namespace, PageToken pageToken) { + PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_VIEWS; + authorizeBasicNamespaceOperationOrThrow(op, namespace); + + if (baseCatalog instanceof IcebergCatalog polarisCatalog) { + return ListTablesResponseWithPageToken.fromPolarisPage(polarisCatalog.listViews(namespace, pageToken)); + } else { + return ListTablesResponseWithPageToken.fromPolarisPage( + PolarisPage.fromData(CatalogHandlers.listTables(baseCatalog, namespace) + .identifiers())); + } + } + public ListTablesResponse listViews(Namespace namespace) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_VIEWS; authorizeBasicNamespaceOperationOrThrow(op, namespace); diff --git a/service/common/src/main/java/org/apache/polaris/service/types/ListTablesResponseWithPageToken.java b/service/common/src/main/java/org/apache/polaris/service/types/ListTablesResponseWithPageToken.java new file mode 100644 index 0000000000..e696f344bc --- /dev/null +++ b/service/common/src/main/java/org/apache/polaris/service/types/ListTablesResponseWithPageToken.java @@ -0,0 +1,73 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.service.types; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.MoreObjects; +import com.google.common.base.Preconditions; +import java.util.List; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.rest.responses.ListTablesResponse; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; + +/** + * Used in lieu of {@link ListTablesResponse} when there may be a {@link PageToken} associated with + * the response. Callers can use this {@link PageToken} to continue the listing operation and obtain + * more results. + */ +public class ListTablesResponseWithPageToken extends ListTablesResponse { + private final PageToken pageToken; + + private final List identifiers; + + public ListTablesResponseWithPageToken(PageToken pageToken, List identifiers) { + this.pageToken = pageToken; + this.identifiers = identifiers; + Preconditions.checkArgument(this.identifiers != null, "Invalid identifier list: null"); + } + + public static ListTablesResponseWithPageToken fromPolarisPage( + PolarisPage polarisPage) { + return new ListTablesResponseWithPageToken(polarisPage.pageToken, polarisPage.data); + } + + @JsonProperty("next-page-token") + public String getPageToken() { + if (pageToken == null) { + return null; + } else { + return pageToken.toString(); + } + } + + @Override + public List identifiers() { + return this.identifiers != null ? this.identifiers : List.of(); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("identifiers", this.identifiers) + .add("pageToken", this.pageToken) + .toString(); + } +} diff --git a/service/common/src/test/java/org/apache/polaris/service/persistence/pagination/PageTokenTest.java b/service/common/src/test/java/org/apache/polaris/service/persistence/pagination/PageTokenTest.java index 93af6dbe03..7581430bc0 100644 --- a/service/common/src/test/java/org/apache/polaris/service/persistence/pagination/PageTokenTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/persistence/pagination/PageTokenTest.java @@ -16,16 +16,15 @@ * specific language governing permissions and limitations * under the License. */ - package org.apache.polaris.service.persistence.pagination; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; - import org.apache.polaris.core.persistence.pagination.OffsetPageToken; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; +import org.apache.polaris.jpa.models.EntityIdPageToken; import org.apache.polaris.jpa.models.ModelEntity; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; @@ -35,173 +34,174 @@ import org.slf4j.LoggerFactory; public class PageTokenTest { - private static final Logger LOGGER = LoggerFactory.getLogger(PageTokenTest.class); - - static Stream> getPageTokenBuilders() { - return Stream.of( - OffsetPageToken.builder(), EntityIdPageToken.builder(), ReadEverythingPageToken.builder()); + private static final Logger LOGGER = LoggerFactory.getLogger(PageTokenTest.class); + + static Stream> getPageTokenBuilders() { + return Stream.of( + OffsetPageToken.builder(), EntityIdPageToken.builder(), ReadEverythingPageToken.builder()); + } + + @Test + void testDoneToken() { + Assertions.assertThat(PageToken.DONE).isNull(); + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testRoundTrips(PageToken.PageTokenBuilder builder) { + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + // Skip ReadEverythingPageToken + return; } - @Test - void testDoneToken() { - Assertions.assertThat(PageToken.DONE).isNull(); + for (int limit : List.of(1, 10, 100, Integer.MAX_VALUE)) { + PageToken token = builder.fromLimit(limit); + Assertions.assertThat(token.pageSize).isEqualTo(limit); + Assertions.assertThat(builder.fromString(token.toString())).isEqualTo(token); } - - @ParameterizedTest - @MethodSource("getPageTokenBuilders") - void testRoundTrips(PageToken.PageTokenBuilder builder) { - if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { - // Skip ReadEverythingPageToken - return; - } - - for (int limit : List.of(1, 10, 100, Integer.MAX_VALUE)) { - PageToken token = builder.fromLimit(limit); - Assertions.assertThat(token.pageSize).isEqualTo(limit); - Assertions.assertThat(builder.fromString(token.toString())).isEqualTo(token); - } - } - - @ParameterizedTest - @MethodSource("getPageTokenBuilders") - void testInvalidLimits(PageToken.PageTokenBuilder builder) { - if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { - // Skip ReadEverythingPageToken - return; - } - - for (int limit : List.of(-1, 0)) { - Assertions.assertThatThrownBy(() -> builder.fromLimit(limit)) - .isInstanceOf(IllegalArgumentException.class); - } - - Assertions.assertThat(builder.fromLimit(null)).isInstanceOf(ReadEverythingPageToken.class); + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testInvalidLimits(PageToken.PageTokenBuilder builder) { + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + // Skip ReadEverythingPageToken + return; } - @ParameterizedTest - @MethodSource("getPageTokenBuilders") - void testStartingTokens(PageToken.PageTokenBuilder builder) { - Assertions.assertThat(builder.fromString("")).isNotNull(); - if (!(builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder)) { - Assertions.assertThat(builder.fromString("")).isNotEqualTo(ReadEverythingPageToken.get()); - } - - Assertions.assertThatThrownBy(() -> builder.fromString(null)) - .isInstanceOf(IllegalArgumentException.class); + for (int limit : List.of(-1, 0)) { + Assertions.assertThatThrownBy(() -> builder.fromLimit(limit)) + .isInstanceOf(IllegalArgumentException.class); } - @ParameterizedTest - @MethodSource("getPageTokenBuilders") - void testPageBuilding(PageToken.PageTokenBuilder builder) { - if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { - // Skip ReadEverythingPageToken - return; - } - - List data = - List.of(ModelEntity.builder().id(1).build(), ModelEntity.builder().id(2).build()); + Assertions.assertThat(builder.fromLimit(null)).isInstanceOf(ReadEverythingPageToken.class); + } - PageToken token = builder.fromLimit(1000); - Assertions.assertThat(token.buildNextPage(data).data).isEqualTo(data); - Assertions.assertThat(token.buildNextPage(data).pageToken).isNull(); + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testStartingTokens(PageToken.PageTokenBuilder builder) { + Assertions.assertThat(builder.fromString("")).isNotNull(); + if (!(builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder)) { + Assertions.assertThat(builder.fromString("")).isNotEqualTo(ReadEverythingPageToken.get()); } - @Test - void testUniquePrefixes() { - Stream> builders = getPageTokenBuilders(); - List prefixes = - builders.map(PageToken.PageTokenBuilder::tokenPrefix).collect(Collectors.toList()); - Assertions.assertThat(prefixes.size()).isEqualTo(prefixes.stream().distinct().count()); - } - - @ParameterizedTest - @MethodSource("getPageTokenBuilders") - void testCrossTokenParsing(PageToken.PageTokenBuilder builder) { - var otherBuilders = getPageTokenBuilders().collect(Collectors.toList()); - for (var otherBuilder : otherBuilders) { - LOGGER.info( - "Testing {} being parsed by {}", - builder.getClass().getSimpleName(), - otherBuilder.getClass().getSimpleName()); - - final PageToken token; - if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { - token = ReadEverythingPageToken.get(); - } else { - token = builder.fromLimit(1234); - } - if (otherBuilder.getClass().equals(builder.getClass())) { - Assertions.assertThat(otherBuilder.fromString(token.toString())).isEqualTo(token); - } else { - Assertions.assertThatThrownBy(() -> otherBuilder.fromString(token.toString())) - .isInstanceOf(IllegalArgumentException.class); - } - } - } + Assertions.assertThatThrownBy(() -> builder.fromString(null)) + .isInstanceOf(IllegalArgumentException.class); + } - @ParameterizedTest - @MethodSource("getPageTokenBuilders") - void testDefaultTokens(PageToken.PageTokenBuilder builder) { - if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { - // Skip ReadEverythingPageToken - return; - } - - PageToken token = builder.fromString(""); - Assertions.assertThat(token.toString()).isNotNull(); - Assertions.assertThat(token.pageSize).isEqualTo(PageToken.DEFAULT_PAGE_SIZE); + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testPageBuilding(PageToken.PageTokenBuilder builder) { + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + // Skip ReadEverythingPageToken + return; } - @Test - void testReadEverythingPageToken() { - PageToken token = ReadEverythingPageToken.get(); - - Assertions.assertThat(token.toString()).isNotNull(); - Assertions.assertThat(token.updated(List.of("anything"))).isEqualTo(PageToken.DONE); - Assertions.assertThat(token.pageSize).isEqualTo(Integer.MAX_VALUE); + List data = + List.of(ModelEntity.builder().id(1).build(), ModelEntity.builder().id(2).build()); + + PageToken token = builder.fromLimit(1000); + Assertions.assertThat(token.buildNextPage(data).data).isEqualTo(data); + Assertions.assertThat(token.buildNextPage(data).pageToken).isNull(); + } + + @Test + void testUniquePrefixes() { + Stream> builders = getPageTokenBuilders(); + List prefixes = + builders.map(PageToken.PageTokenBuilder::tokenPrefix).collect(Collectors.toList()); + Assertions.assertThat(prefixes.size()).isEqualTo(prefixes.stream().distinct().count()); + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testCrossTokenParsing(PageToken.PageTokenBuilder builder) { + var otherBuilders = getPageTokenBuilders().collect(Collectors.toList()); + for (var otherBuilder : otherBuilders) { + LOGGER.info( + "Testing {} being parsed by {}", + builder.getClass().getSimpleName(), + otherBuilder.getClass().getSimpleName()); + + final PageToken token; + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + token = ReadEverythingPageToken.get(); + } else { + token = builder.fromLimit(1234); + } + if (otherBuilder.getClass().equals(builder.getClass())) { + Assertions.assertThat(otherBuilder.fromString(token.toString())).isEqualTo(token); + } else { + Assertions.assertThatThrownBy(() -> otherBuilder.fromString(token.toString())) + .isInstanceOf(IllegalArgumentException.class); + } } - - @Test - void testOffsetPageToken() { - OffsetPageToken token = (OffsetPageToken) OffsetPageToken.builder().fromLimit(2); - - Assertions.assertThat(token).isInstanceOf(OffsetPageToken.class); - Assertions.assertThat(token.offset).isEqualTo(0); - - List data = List.of("some", "data"); - var page = token.buildNextPage(data); - Assertions.assertThat(page.pageToken).isNotNull(); - Assertions.assertThat(page.pageToken).isInstanceOf(OffsetPageToken.class); - Assertions.assertThat(page.pageToken.pageSize).isEqualTo(2); - Assertions.assertThat(((OffsetPageToken) page.pageToken).offset).isEqualTo(2); - Assertions.assertThat(page.data).isEqualTo(data); - - Assertions.assertThat(OffsetPageToken.builder().fromString(page.pageToken.toString())) - .isEqualTo(page.pageToken); + } + + @ParameterizedTest + @MethodSource("getPageTokenBuilders") + void testDefaultTokens(PageToken.PageTokenBuilder builder) { + if (builder instanceof ReadEverythingPageToken.ReadEverythingPageTokenBuilder) { + // Skip ReadEverythingPageToken + return; } - @Test - void testEntityIdPageToken() { - EntityIdPageToken token = (EntityIdPageToken) EntityIdPageToken.builder().fromLimit(2); - - Assertions.assertThat(token).isInstanceOf(EntityIdPageToken.class); - Assertions.assertThat(token.id).isEqualTo(-1L); - - List badData = List.of("some", "data"); - Assertions.assertThatThrownBy(() -> token.buildNextPage(badData)) - .isInstanceOf(IllegalArgumentException.class); - - List data = - List.of(ModelEntity.builder().id(101).build(), ModelEntity.builder().id(102).build()); - var page = token.buildNextPage(data); - - Assertions.assertThat(page.pageToken).isNotNull(); - Assertions.assertThat(page.pageToken).isInstanceOf(EntityIdPageToken.class); - Assertions.assertThat(page.pageToken.pageSize).isEqualTo(2); - Assertions.assertThat(((EntityIdPageToken) page.pageToken).id).isEqualTo(102); - Assertions.assertThat(page.data).isEqualTo(data); - - Assertions.assertThat(EntityIdPageToken.builder().fromString(page.pageToken.toString())) - .isEqualTo(page.pageToken); - } + PageToken token = builder.fromString(""); + Assertions.assertThat(token.toString()).isNotNull(); + Assertions.assertThat(token.pageSize).isEqualTo(PageToken.DEFAULT_PAGE_SIZE); + } + + @Test + void testReadEverythingPageToken() { + PageToken token = ReadEverythingPageToken.get(); + + Assertions.assertThat(token.toString()).isNotNull(); + Assertions.assertThat(token.buildNextPage(List.of("anything")).pageToken) + .isEqualTo(PageToken.DONE); + Assertions.assertThat(token.pageSize).isEqualTo(Integer.MAX_VALUE); + } + + @Test + void testOffsetPageToken() { + OffsetPageToken token = (OffsetPageToken) OffsetPageToken.builder().fromLimit(2); + + Assertions.assertThat(token).isInstanceOf(OffsetPageToken.class); + Assertions.assertThat(token.offset).isEqualTo(0); + + List data = List.of("some", "data"); + var page = token.buildNextPage(data); + Assertions.assertThat(page.pageToken).isNotNull(); + Assertions.assertThat(page.pageToken).isInstanceOf(OffsetPageToken.class); + Assertions.assertThat(page.pageToken.pageSize).isEqualTo(2); + Assertions.assertThat(((OffsetPageToken) page.pageToken).offset).isEqualTo(2); + Assertions.assertThat(page.data).isEqualTo(data); + + Assertions.assertThat(OffsetPageToken.builder().fromString(page.pageToken.toString())) + .isEqualTo(page.pageToken); + } + + @Test + void testEntityIdPageToken() { + EntityIdPageToken token = (EntityIdPageToken) EntityIdPageToken.builder().fromLimit(2); + + Assertions.assertThat(token).isInstanceOf(EntityIdPageToken.class); + Assertions.assertThat(token.id).isEqualTo(-1L); + + List badData = List.of("some", "data"); + Assertions.assertThatThrownBy(() -> token.buildNextPage(badData)) + .isInstanceOf(IllegalArgumentException.class); + + List data = + List.of(ModelEntity.builder().id(101).build(), ModelEntity.builder().id(102).build()); + var page = token.buildNextPage(data); + + Assertions.assertThat(page.pageToken).isNotNull(); + Assertions.assertThat(page.pageToken).isInstanceOf(EntityIdPageToken.class); + Assertions.assertThat(page.pageToken.pageSize).isEqualTo(2); + Assertions.assertThat(((EntityIdPageToken) page.pageToken).id).isEqualTo(102); + Assertions.assertThat(page.data).isEqualTo(data); + + Assertions.assertThat(EntityIdPageToken.builder().fromString(page.pageToken.toString())) + .isEqualTo(page.pageToken); + } } From 6d7c59b90b4c7aa016df2646fe9cb0d476960564 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 14:00:20 -0700 Subject: [PATCH 12/23] continuing rebase --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 70 ++++++++---- .../eclipselink/PolarisEclipseLinkStore.java | 30 +++++- .../AtomicOperationMetaStoreManager.java | 47 ++++---- .../core/persistence/BasePersistence.java | 19 ++-- .../persistence/PolarisMetaStoreManager.java | 9 +- .../TransactionWorkspaceMetaStoreManager.java | 6 +- .../dao/entity/EntitiesResult.java | 25 ++++- .../dao/entity/ListEntitiesResult.java | 28 ++++- .../AbstractTransactionalPersistence.java | 26 +++-- .../TransactionalMetaStoreManagerImpl.java | 66 +++++++----- .../TransactionalPersistence.java | 18 ++-- .../TreeMapTransactionalPersistenceImpl.java | 62 +++++++---- .../service/admin/PolarisAdminService.java | 13 ++- .../catalog/generic/GenericTableCatalog.java | 4 +- .../catalog/iceberg/IcebergCatalog.java | 101 ++++++++++++++---- .../iceberg/IcebergCatalogAdapter.java | 13 ++- .../iceberg/IcebergCatalogHandlerWrapper.java | 13 +-- .../ListTablesResponseWithPageToken.java | 59 +++++----- 18 files changed, 411 insertions(+), 198 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 7a377fde24..e9090dbc5a 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -53,6 +53,7 @@ import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; import org.apache.polaris.core.persistence.transactional.AbstractTransactionalPersistence; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -412,29 +413,30 @@ public List lookupEntityActiveBatchInCurrentTxn( /** {@inheritDoc} */ @Override - public @Nonnull List listEntitiesInCurrentTxn( + public @Nonnull PolarisPage listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, - @Nonnull PolarisEntityType entityType) { + @Nonnull PolarisEntityType entityType, + @Nonnull PageToken pageToken) { return this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue()); + callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageToken); } @Override - public @Nonnull List listEntitiesInCurrentTxn( + public @Nonnull PolarisPage listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter) { + @Nonnull Predicate entityFilter, + @Nonnull PageToken pageToken) { // full range scan under the parent for that type return this.listEntitiesInCurrentTxn( callCtx, catalogId, parentId, entityType, - Integer.MAX_VALUE, entityFilter, entity -> new EntityNameLookupRecord( @@ -443,27 +445,57 @@ public List lookupEntityActiveBatchInCurrentTxn( entity.getParentId(), entity.getName(), entity.getTypeCode(), - entity.getSubTypeCode())); + entity.getSubTypeCode()), + pageToken); } @Override - public @Nonnull List listEntitiesInCurrentTxn( + public @Nonnull PolarisPage listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - int limit, @Nonnull Predicate entityFilter, - @Nonnull Function transformer) { - // full range scan under the parent for that type - return this.store - .lookupFullEntitiesActive(localSession.get(), catalogId, parentId, entityType) - .stream() - .map(ModelEntity::toEntity) - .filter(entityFilter) - .limit(limit) - .map(transformer) - .collect(Collectors.toList()); + @Nonnull Function transformer, + @Nonnull PageToken pageToken) { + List data; + if (entityFilter.equals(Predicates.alwaysTrue())) { + // In this case, we can push the filter down into the query + data = + this.store + .lookupFullEntitiesActive( + localSession.get(), catalogId, parentId, entityType, pageToken) + .stream() + .map(ModelEntity::toEntity) + .filter(entityFilter) + .map(transformer) + .collect(Collectors.toList()); + } else { + // In this case, we cannot push the filter down into the query. We must therefore remove + // the page size limit from the PageToken and filter on the client side. + // TODO Implement a generic predicate that can be pushed down into different metastores + PageToken unlimitedPageSizeToken = pageToken.withPageSize(Integer.MAX_VALUE); + List rawData = + this.store.lookupFullEntitiesActive( + localSession.get(), catalogId, parentId, entityType, unlimitedPageSizeToken); + if (pageToken.pageSize < Integer.MAX_VALUE && rawData.size() > pageToken.pageSize) { + LOGGER.info( + "A page token could not be respected due to a predicate. " + + "{} records were read but the client was asked to return {}.", + rawData.size(), + pageToken.pageSize); + } + + data = + rawData.stream() + .map(ModelEntity::toEntity) + .filter(entityFilter) + .limit(pageToken.pageSize) + .map(transformer) + .collect(Collectors.toList()); + } + + return pageToken.buildNextPage(data); } /** {@inheritDoc} */ diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java index bfc83ae373..914f55b7b4 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java @@ -35,6 +35,9 @@ import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; +import org.apache.polaris.jpa.models.EntityIdPageToken; import org.apache.polaris.jpa.models.ModelEntity; import org.apache.polaris.jpa.models.ModelEntityActive; import org.apache.polaris.jpa.models.ModelEntityChangeTracking; @@ -278,21 +281,40 @@ long countActiveChildEntities( } List lookupFullEntitiesActive( - EntityManager session, long catalogId, long parentId, @Nonnull PolarisEntityType entityType) { + EntityManager session, + long catalogId, + long parentId, + @Nonnull PolarisEntityType entityType, + PageToken pageToken) { diagnosticServices.check(session != null, "session_is_null"); + diagnosticServices.check( + (pageToken instanceof EntityIdPageToken || pageToken instanceof ReadEverythingPageToken), + "unexpected_page_token"); checkInitialized(); // Currently check against ENTITIES not joining with ENTITIES_ACTIVE String hql = - "SELECT m from ModelEntity m where m.catalogId=:catalogId and m.parentId=:parentId and m.typeCode=:typeCode"; + "SELECT m from ModelEntity m " + + "where m.catalogId=:catalogId and m.parentId=:parentId and m.typeCode=:typeCode and m.id > :tokenId"; + + if (pageToken instanceof EntityIdPageToken) { + hql += " order by m.id asc"; + } TypedQuery query = session .createQuery(hql, ModelEntity.class) .setParameter("catalogId", catalogId) .setParameter("parentId", parentId) - .setParameter("typeCode", entityType.getCode()); - + .setParameter("typeCode", entityType.getCode()) + .setParameter("tokenId", -1L); + + if (pageToken instanceof EntityIdPageToken) { + query = + query + .setParameter("tokenId", ((EntityIdPageToken) pageToken).id) + .setMaxResults(pageToken.pageSize); + } return query.getResultList(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 7dcc9f0ac6..bcb23e4ff8 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -61,6 +61,8 @@ import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; import org.apache.polaris.core.storage.PolarisCredentialProperty; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; @@ -662,7 +664,8 @@ private void revokeGrantRecord( @Nonnull PolarisCallContext callCtx, @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, - @Nonnull PolarisEntitySubType entitySubType) { + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken) { // get meta store we should be using BasePersistence ms = callCtx.getMetaStore(); @@ -674,15 +677,16 @@ private void revokeGrantRecord( catalogPath == null || catalogPath.size() == 0 ? 0l : catalogPath.get(catalogPath.size() - 1).getId(); - List toreturnList = - ms.listEntities(callCtx, catalogId, parentId, entityType); + PolarisPage resultPage = + ms.listEntities(callCtx, catalogId, parentId, entityType, pageToken); // prune the returned list with only entities matching the entity subtype if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) { - toreturnList = - toreturnList.stream() - .filter(rec -> rec.getSubTypeCode() == entitySubType.getCode()) - .collect(Collectors.toList()); + resultPage = + pageToken.buildNextPage( + resultPage.data.stream() + .filter(rec -> rec.getSubTypeCode() == entitySubType.getCode()) + .collect(Collectors.toList())); } // TODO: Use post-validation to enforce consistent view against catalogPath. In the @@ -692,7 +696,7 @@ private void revokeGrantRecord( // in-flight request (the cache-based resolution follows a different path entirely). // done - return new ListEntitiesResult(toreturnList); + return ListEntitiesResult.fromPolarisPage(resultPage); } /** {@inheritDoc} */ @@ -1151,13 +1155,14 @@ private void revokeGrantRecord( // get the list of catalog roles, at most 2 List catalogRoles = ms.listEntities( - callCtx, - catalogId, - catalogId, - PolarisEntityType.CATALOG_ROLE, - 2, - entity -> true, - Function.identity()); + callCtx, + catalogId, + catalogId, + PolarisEntityType.CATALOG_ROLE, + entity -> true, + Function.identity(), + ms.pageTokenBuilder().fromLimit(2)) + .data; // if we have 2, we cannot drop the catalog. If only one left, better be the admin role if (catalogRoles.size() > 1) { @@ -1451,17 +1456,16 @@ private void revokeGrantRecord( @Override public @Nonnull EntitiesResult loadTasks( - @Nonnull PolarisCallContext callCtx, String executorId, int limit) { + @Nonnull PolarisCallContext callCtx, String executorId, PageToken pageToken) { BasePersistence ms = callCtx.getMetaStore(); // find all available tasks - List availableTasks = + PolarisPage availableTasks = ms.listEntities( callCtx, PolarisEntityConstants.getRootEntityId(), PolarisEntityConstants.getRootEntityId(), PolarisEntityType.TASK, - limit, entity -> { PolarisObjectMapperUtil.TaskExecutionState taskState = PolarisObjectMapperUtil.parseTaskState(entity); @@ -1476,11 +1480,12 @@ private void revokeGrantRecord( || taskState.executor == null || callCtx.getClock().millis() - taskState.lastAttemptStartTime > taskAgeTimeout; }, - Function.identity()); + Function.identity(), + pageToken); List loadedTasks = new ArrayList<>(); final AtomicInteger failedLeaseCount = new AtomicInteger(0); - availableTasks.forEach( + availableTasks.data.forEach( task -> { PolarisBaseEntity updatedTask = new PolarisBaseEntity(task); Map properties = @@ -1517,7 +1522,7 @@ private void revokeGrantRecord( throw new RetryOnConcurrencyException( "Failed to lease any of %s tasks due to concurrent leases", failedLeaseCount.get()); } - return new EntitiesResult(loadedTasks); + return EntitiesResult.fromPolarisPage(PolarisPage.fromData(loadedTasks)); } /** {@inheritDoc} */ diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index 0f5be70805..d3a485f74e 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -32,6 +32,7 @@ import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; /** * Interface to the Polaris persistence backend, with which to persist and retrieve all the data @@ -266,14 +267,16 @@ List lookupEntityVersions( * @param catalogId catalog id for that entity, NULL_ID if the entity is top-level * @param parentId id of the parent, can be the special 0 value representing the root entity * @param entityType type of entities to list + * @param pageToken the token to start listing after * @return the list of entities for the specified list operation */ @Nonnull - List listEntities( + PolarisPage listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, - @Nonnull PolarisEntityType entityType); + @Nonnull PolarisEntityType entityType, + @Nonnull PageToken pageToken); /** * List entities where some predicate returns true @@ -284,15 +287,17 @@ List listEntities( * @param entityType type of entities to list * @param entityFilter the filter to be applied to each entity. Only entities where the predicate * returns true are returned in the list + * @param pageToken the token to start listing after * @return the list of entities for which the predicate returns true */ @Nonnull - List listEntities( + PolarisPage listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter); + @Nonnull Predicate entityFilter, + @Nonnull PageToken pageToken); /** * List entities where some predicate returns true and transform the entities with a function @@ -309,14 +314,14 @@ List listEntities( * @return the list of entities for which the predicate returns true */ @Nonnull - List listEntities( + PolarisPage listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - int limit, @Nonnull Predicate entityFilter, - @Nonnull Function transformer); + @Nonnull Function transformer, + PageToken pageToken); /** * Lookup the current entityGrantRecordsVersion for the specified entity. That version is changed diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java index cc082bc27b..1480fa43cc 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java @@ -42,6 +42,7 @@ import org.apache.polaris.core.persistence.dao.entity.GenerateEntityIdResult; import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; +import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.storage.PolarisCredentialVendor; /** @@ -116,7 +117,8 @@ ListEntitiesResult listEntities( @Nonnull PolarisCallContext callCtx, @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, - @Nonnull PolarisEntitySubType entitySubType); + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken); /** * Generate a new unique id that can be used by the Polaris client when it needs to create a new @@ -296,11 +298,12 @@ EntityResult loadEntity( * * @param callCtx call context * @param executorId executor id - * @param limit limit + * @param pageToken page token to start after * @return list of tasks to be completed */ @Nonnull - EntitiesResult loadTasks(@Nonnull PolarisCallContext callCtx, String executorId, int limit); + EntitiesResult loadTasks( + @Nonnull PolarisCallContext callCtx, String executorId, PageToken pageToken); /** * Load change tracking information for a set of entities in one single shot and return for each diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 4f26c51fa6..2abf63dc53 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -49,6 +49,7 @@ import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult; +import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.storage.PolarisStorageActions; /** @@ -116,7 +117,8 @@ public ListEntitiesResult listEntities( @Nonnull PolarisCallContext callCtx, @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, - @Nonnull PolarisEntitySubType entitySubType) { + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken) { callCtx.getDiagServices().fail("illegal_method_in_transaction_workspace", "listEntities"); return null; } @@ -318,7 +320,7 @@ public EntityResult loadEntity( @Override public EntitiesResult loadTasks( - @Nonnull PolarisCallContext callCtx, String executorId, int limit) { + @Nonnull PolarisCallContext callCtx, String executorId, PageToken pageToken) { callCtx.getDiagServices().fail("illegal_method_in_transaction_workspace", "loadTasks"); return null; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/EntitiesResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/EntitiesResult.java index 70d9edcf58..3e311b9102 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/EntitiesResult.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/EntitiesResult.java @@ -23,13 +23,21 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.util.List; +import java.util.Optional; import org.apache.polaris.core.entity.PolarisBaseEntity; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; /** a set of returned entities result */ public class EntitiesResult extends BaseResult { // null if not success. Else the list of entities being returned private final List entities; + private final Optional pageTokenOpt; + + public static EntitiesResult fromPolarisPage(PolarisPage polarisPage) { + return new EntitiesResult(polarisPage.data, Optional.ofNullable(polarisPage.pageToken)); + } /** * Constructor for an error @@ -40,6 +48,11 @@ public class EntitiesResult extends BaseResult { public EntitiesResult(@Nonnull ReturnStatus errorStatus, @Nullable String extraInformation) { super(errorStatus, extraInformation); this.entities = null; + this.pageTokenOpt = Optional.empty(); + } + + public EntitiesResult(@Nonnull List entities) { + this(entities, Optional.empty()); } /** @@ -47,21 +60,29 @@ public EntitiesResult(@Nonnull ReturnStatus errorStatus, @Nullable String extraI * * @param entities list of entities being returned, implies success */ - public EntitiesResult(@Nonnull List entities) { + public EntitiesResult( + @Nonnull List entities, @Nonnull Optional pageTokenOpt) { super(ReturnStatus.SUCCESS); this.entities = entities; + this.pageTokenOpt = pageTokenOpt; } @JsonCreator private EntitiesResult( @JsonProperty("returnStatus") @Nonnull ReturnStatus returnStatus, @JsonProperty("extraInformation") String extraInformation, - @JsonProperty("entities") List entities) { + @JsonProperty("entities") List entities, + @JsonProperty("pageToken") Optional pageTokenOpt) { super(returnStatus, extraInformation); this.entities = entities; + this.pageTokenOpt = pageTokenOpt; } public List getEntities() { return entities; } + + public Optional getPageToken() { + return pageTokenOpt; + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ListEntitiesResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ListEntitiesResult.java index bc51f4dab5..f43985ec82 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ListEntitiesResult.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ListEntitiesResult.java @@ -23,13 +23,23 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.util.List; +import java.util.Optional; import org.apache.polaris.core.entity.EntityNameLookupRecord; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; /** the return the result for a list entities call */ public class ListEntitiesResult extends BaseResult { // null if not success. Else the list of entities being returned private final List entities; + private final Optional pageTokenOpt; + + /** Create a {@link ListEntitiesResult} from a {@link PolarisPage} */ + public static ListEntitiesResult fromPolarisPage( + PolarisPage polarisPage) { + return new ListEntitiesResult(polarisPage.data, Optional.ofNullable(polarisPage.pageToken)); + } /** * Constructor for an error @@ -37,9 +47,13 @@ public class ListEntitiesResult extends BaseResult { * @param errorCode error code, cannot be SUCCESS * @param extraInformation extra information */ - public ListEntitiesResult(@Nonnull ReturnStatus errorCode, @Nullable String extraInformation) { + public ListEntitiesResult( + @Nonnull ReturnStatus errorCode, + @Nullable String extraInformation, + @Nonnull Optional pageTokenOpt) { super(errorCode, extraInformation); this.entities = null; + this.pageTokenOpt = pageTokenOpt; } /** @@ -47,21 +61,29 @@ public ListEntitiesResult(@Nonnull ReturnStatus errorCode, @Nullable String extr * * @param entities list of entities being returned, implies success */ - public ListEntitiesResult(@Nonnull List entities) { + public ListEntitiesResult( + @Nonnull List entities, @Nullable Optional pageTokenOpt) { super(ReturnStatus.SUCCESS); this.entities = entities; + this.pageTokenOpt = pageTokenOpt; } @JsonCreator private ListEntitiesResult( @JsonProperty("returnStatus") @Nonnull ReturnStatus returnStatus, @JsonProperty("extraInformation") String extraInformation, - @JsonProperty("entities") List entities) { + @JsonProperty("entities") List entities, + @JsonProperty("pageToken") Optional pageTokenOpt) { super(returnStatus, extraInformation); this.entities = entities; + this.pageTokenOpt = pageTokenOpt; } public List getEntities() { return entities; } + + public Optional getPageToken() { + return pageTokenOpt; + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java index c673c48499..fde000cd77 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java @@ -35,6 +35,8 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.EntityAlreadyExistsException; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -348,46 +350,50 @@ public List lookupEntityVersions( /** {@inheritDoc} */ @Override @Nonnull - public List listEntities( + public PolarisPage listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, - @Nonnull PolarisEntityType entityType) { + @Nonnull PolarisEntityType entityType, + @Nonnull PageToken pageToken) { return runInReadTransaction( - callCtx, () -> this.listEntitiesInCurrentTxn(callCtx, catalogId, parentId, entityType)); + callCtx, + () -> this.listEntitiesInCurrentTxn(callCtx, catalogId, parentId, entityType, pageToken)); } /** {@inheritDoc} */ @Override @Nonnull - public List listEntities( + public PolarisPage listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter) { + @Nonnull Predicate entityFilter, + @Nonnull PageToken pageToken) { return runInReadTransaction( callCtx, () -> - this.listEntitiesInCurrentTxn(callCtx, catalogId, parentId, entityType, entityFilter)); + this.listEntitiesInCurrentTxn( + callCtx, catalogId, parentId, entityType, entityFilter, pageToken)); } /** {@inheritDoc} */ @Override @Nonnull - public List listEntities( + public PolarisPage listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - int limit, @Nonnull Predicate entityFilter, - @Nonnull Function transformer) { + @Nonnull Function transformer, + @Nonnull PageToken pageToken) { return runInReadTransaction( callCtx, () -> this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, limit, entityFilter, transformer)); + callCtx, catalogId, parentId, entityType, entityFilter, transformer, pageToken)); } /** {@inheritDoc} */ diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index cf1e54b535..67f562956e 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -62,6 +63,8 @@ import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; import org.apache.polaris.core.storage.PolarisCredentialProperty; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; @@ -651,37 +654,41 @@ private void bootstrapPolarisService( } /** - * See {@link #listEntities(PolarisCallContext, List, PolarisEntityType, PolarisEntitySubType)} + * See {@link #listEntities(PolarisCallContext, List, PolarisEntityType, PolarisEntitySubType, + * PageToken)} */ private @Nonnull ListEntitiesResult listEntities( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, - @Nonnull PolarisEntitySubType entitySubType) { + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken) { // first resolve again the catalogPath to that entity PolarisEntityResolver resolver = new PolarisEntityResolver(callCtx, ms, catalogPath); // return if we failed to resolve if (resolver.isFailure()) { - return new ListEntitiesResult(BaseResult.ReturnStatus.CATALOG_PATH_CANNOT_BE_RESOLVED, null); + return new ListEntitiesResult( + BaseResult.ReturnStatus.CATALOG_PATH_CANNOT_BE_RESOLVED, null, Optional.empty()); } // return list of active entities - List toreturnList = + PolarisPage resultPage = ms.listEntitiesInCurrentTxn( - callCtx, resolver.getCatalogIdOrNull(), resolver.getParentId(), entityType); + callCtx, resolver.getCatalogIdOrNull(), resolver.getParentId(), entityType, pageToken); // prune the returned list with only entities matching the entity subtype if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) { - toreturnList = - toreturnList.stream() - .filter(rec -> rec.getSubTypeCode() == entitySubType.getCode()) - .collect(Collectors.toList()); + resultPage = + pageToken.buildNextPage( + resultPage.data.stream() + .filter(rec -> rec.getSubTypeCode() == entitySubType.getCode()) + .collect(Collectors.toList())); } // done - return new ListEntitiesResult(toreturnList); + return ListEntitiesResult.fromPolarisPage(resultPage); } /** {@inheritDoc} */ @@ -690,13 +697,15 @@ private void bootstrapPolarisService( @Nonnull PolarisCallContext callCtx, @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, - @Nonnull PolarisEntitySubType entitySubType) { + @Nonnull PolarisEntitySubType entitySubType, + @Nonnull PageToken pageToken) { // get meta store we should be using TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); // run operation in a read transaction return ms.runInReadTransaction( - callCtx, () -> listEntities(callCtx, ms, catalogPath, entityType, entitySubType)); + callCtx, + () -> listEntities(callCtx, ms, catalogPath, entityType, entitySubType, pageToken)); } /** {@link #createPrincipal(PolarisCallContext, PolarisBaseEntity)} */ @@ -1333,13 +1342,14 @@ private void bootstrapPolarisService( // get the list of catalog roles, at most 2 List catalogRoles = ms.listEntitiesInCurrentTxn( - callCtx, - catalogId, - catalogId, - PolarisEntityType.CATALOG_ROLE, - 2, - entity -> true, - Function.identity()); + callCtx, + catalogId, + catalogId, + PolarisEntityType.CATALOG_ROLE, + entity -> true, + Function.identity(), + ms.pageTokenBuilder().fromLimit(2)) + .data; // if we have 2, we cannot drop the catalog. If only one left, better be the admin role if (catalogRoles.size() > 1) { @@ -1881,21 +1891,20 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( () -> this.loadEntity(callCtx, ms, entityCatalogId, entityId, entityType.getCode())); } - /** Refer to {@link #loadTasks(PolarisCallContext, String, int)} */ + /** Refer to {@link #loadTasks(PolarisCallContext, String, PageToken)} */ private @Nonnull EntitiesResult loadTasks( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, String executorId, - int limit) { + PageToken pageToken) { // find all available tasks - List availableTasks = + PolarisPage availableTasks = ms.listEntitiesInCurrentTxn( callCtx, PolarisEntityConstants.getRootEntityId(), PolarisEntityConstants.getRootEntityId(), PolarisEntityType.TASK, - limit, entity -> { PolarisObjectMapperUtil.TaskExecutionState taskState = PolarisObjectMapperUtil.parseTaskState(entity); @@ -1910,10 +1919,11 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( || taskState.executor == null || callCtx.getClock().millis() - taskState.lastAttemptStartTime > taskAgeTimeout; }, - Function.identity()); + Function.identity(), + pageToken); List loadedTasks = new ArrayList<>(); - availableTasks.forEach( + availableTasks.data.forEach( task -> { // Make a copy to avoid mutating someone else's reference. // TODO: Refactor into immutable/Builder pattern. @@ -1944,14 +1954,14 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( result.getReturnStatus(), result.getExtraInformation()); } }); - return new EntitiesResult(loadedTasks); + return EntitiesResult.fromPolarisPage(PolarisPage.fromData(loadedTasks)); } @Override public @Nonnull EntitiesResult loadTasks( - @Nonnull PolarisCallContext callCtx, String executorId, int limit) { + @Nonnull PolarisCallContext callCtx, String executorId, PageToken pageToken) { TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); - return ms.runInTransaction(callCtx, () -> this.loadTasks(callCtx, ms, executorId, limit)); + return ms.runInTransaction(callCtx, () -> this.loadTasks(callCtx, ms, executorId, pageToken)); } /** {@inheritDoc} */ diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java index f3d3842bb2..5794a6243b 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java @@ -36,6 +36,8 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.IntegrationPersistence; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -199,31 +201,33 @@ List lookupEntityVersionsInCurrentTxn( /** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */ @Nonnull - List listEntitiesInCurrentTxn( + PolarisPage listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, - @Nonnull PolarisEntityType entityType); + @Nonnull PolarisEntityType entityType, + @Nonnull PageToken pageToken); /** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */ @Nonnull - List listEntitiesInCurrentTxn( + PolarisPage listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter); + @Nonnull Predicate entityFilter, + @Nonnull PageToken pageToken); /** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */ @Nonnull - List listEntitiesInCurrentTxn( + PolarisPage listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - int limit, @Nonnull Predicate entityFilter, - @Nonnull Function transformer); + @Nonnull Function transformer, + @Nonnull PageToken pageToken); /** * See {@link org.apache.polaris.core.persistence.BasePersistence#lookupEntityGrantRecordsVersion} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 7b849514b9..44ef9422bb 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -21,11 +21,13 @@ import com.google.common.base.Predicates; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.util.Comparator; import java.util.List; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.entity.EntityNameLookupRecord; import org.apache.polaris.core.entity.PolarisBaseEntity; @@ -40,6 +42,8 @@ import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; import org.apache.polaris.core.persistence.pagination.OffsetPageToken; import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; @@ -297,29 +301,30 @@ public List lookupEntityActiveBatchInCurrentTxn( /** {@inheritDoc} */ @Override - public @Nonnull List listEntitiesInCurrentTxn( + public @Nonnull PolarisPage listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, - @Nonnull PolarisEntityType entityType) { + @Nonnull PolarisEntityType entityType, + @Nonnull PageToken pageToken) { return this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue()); + callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageToken); } @Override - public @Nonnull List listEntitiesInCurrentTxn( + public @Nonnull PolarisPage listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter) { + @Nonnull Predicate entityFilter, + @Nonnull PageToken pageToken) { // full range scan under the parent for that type return this.listEntitiesInCurrentTxn( callCtx, catalogId, parentId, entityType, - Integer.MAX_VALUE, entityFilter, entity -> new EntityNameLookupRecord( @@ -328,31 +333,42 @@ public List lookupEntityActiveBatchInCurrentTxn( entity.getParentId(), entity.getName(), entity.getTypeCode(), - entity.getSubTypeCode())); + entity.getSubTypeCode()), + ReadEverythingPageToken.get()); } @Override - public @Nonnull List listEntitiesInCurrentTxn( + public @Nonnull PolarisPage listEntitiesInCurrentTxn( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - int limit, @Nonnull Predicate entityFilter, - @Nonnull Function transformer) { - // full range scan under the parent for that type - return this.store - .getSliceEntitiesActive() - .readRange(this.store.buildPrefixKeyComposite(catalogId, parentId, entityType.getCode())) - .stream() - .map( - nameRecord -> - this.lookupEntityInCurrentTxn( - callCtx, catalogId, nameRecord.getId(), entityType.getCode())) - .filter(entityFilter) - .limit(limit) - .map(transformer) - .collect(Collectors.toList()); + @Nonnull Function transformer, + @Nonnull PageToken pageToken) { + if (!(pageToken instanceof ReadEverythingPageToken) + && !(pageToken instanceof OffsetPageToken)) { + throw new IllegalArgumentException("Unexpected pageToken: " + pageToken); + } + + Stream partialResults = + this.store + .getSliceEntitiesActive() + .readRange( + this.store.buildPrefixKeyComposite(catalogId, parentId, entityType.getCode())) + .stream() + .filter(entityFilter); + + if (pageToken instanceof OffsetPageToken) { + partialResults = + partialResults + .sorted(Comparator.comparingLong(PolarisEntityCore::getId)) + .skip(((OffsetPageToken) pageToken).offset) + .limit(pageToken.pageSize); + } + + List entities = partialResults.map(transformer).collect(Collectors.toList()); + return pageToken.buildNextPage(entities); } /** {@inheritDoc} */ diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index bedb10165c..b72ce918e6 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -88,6 +88,7 @@ import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; @@ -759,7 +760,8 @@ private List listCatalogsUnsafe() { getCurrentPolarisContext(), null, PolarisEntityType.CATALOG, - PolarisEntitySubType.ANY_SUBTYPE) + PolarisEntitySubType.ANY_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities() .stream() .map( @@ -925,7 +927,8 @@ public List listPrincipals() { getCurrentPolarisContext(), null, PolarisEntityType.PRINCIPAL, - PolarisEntitySubType.NULL_SUBTYPE) + PolarisEntitySubType.NULL_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities() .stream() .map( @@ -1034,7 +1037,8 @@ public List listPrincipalRoles() { getCurrentPolarisContext(), null, PolarisEntityType.PRINCIPAL_ROLE, - PolarisEntitySubType.NULL_SUBTYPE) + PolarisEntitySubType.NULL_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities() .stream() .map( @@ -1162,7 +1166,8 @@ public List listCatalogRoles(String catalogName) { getCurrentPolarisContext(), PolarisEntity.toCoreList(List.of(catalogEntity)), PolarisEntityType.CATALOG_ROLE, - PolarisEntitySubType.NULL_SUBTYPE) + PolarisEntitySubType.NULL_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities() .stream() .map( diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java index 322b7cb0db..b005b55d4b 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java @@ -39,6 +39,7 @@ import org.apache.polaris.core.persistence.dao.entity.BaseResult; import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.task.TaskExecutor; @@ -209,7 +210,8 @@ public List listGenericTables(Namespace namespace) { this.callContext.getPolarisCallContext(), PolarisEntity.toCoreList(catalogPath), PolarisEntityType.GENERIC_TABLE, - PolarisEntitySubType.ANY_SUBTYPE) + PolarisEntitySubType.ANY_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities()); return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities); } 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 9af0d977a6..d692f462ef 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 @@ -99,6 +99,8 @@ import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult; import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.apache.polaris.core.persistence.resolver.ResolverPath; @@ -459,14 +461,32 @@ public boolean dropTable(TableIdentifier tableIdentifier, boolean purge) { return true; } + /** Check whether pagination is enabled for list operations */ + private boolean paginationEnabled() { + return callContext + .getPolarisCallContext() + .getConfigurationStore() + .getConfiguration( + callContext.getPolarisCallContext(), + catalogEntity, + FeatureConfiguration.LIST_PAGINATION_ENABLED); + } + @Override public List listTables(Namespace namespace) { + return listTables(namespace, ReadEverythingPageToken.get()).data; + } + + public PolarisPage listTables(Namespace namespace, PageToken pageToken) { if (!namespaceExists(namespace) && !namespace.isEmpty()) { throw new NoSuchNamespaceException( "Cannot list tables for namespace. Namespace does not exist: %s", namespace); } + if (!paginationEnabled()) { + pageToken = ReadEverythingPageToken.get(); + } - return listTableLike(PolarisEntitySubType.TABLE, namespace); + return listTableLike(PolarisEntitySubType.TABLE, namespace, pageToken); } @Override @@ -770,22 +790,40 @@ public List listNamespaces() { @Override public List listNamespaces(Namespace namespace) throws NoSuchNamespaceException { + return listNamespaces(namespace, ReadEverythingPageToken.get()).data; + } + + public PolarisPage listNamespaces(PageToken pageToken) + throws NoSuchNamespaceException { + return listNamespaces(Namespace.empty(), pageToken); + } + + public PolarisPage listNamespaces(Namespace namespace, PageToken pageToken) + throws NoSuchNamespaceException { PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); if (resolvedEntities == null) { throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); } + if (!paginationEnabled()) { + pageToken = ReadEverythingPageToken.get(); + } List catalogPath = resolvedEntities.getRawFullPath(); + ListEntitiesResult listResult = + getMetaStoreManager() + .listEntities( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + PolarisEntityType.NAMESPACE, + PolarisEntitySubType.NULL_SUBTYPE, + pageToken); List entities = - PolarisEntity.toNameAndIdList( - getMetaStoreManager() - .listEntities( - getCurrentPolarisContext(), - PolarisEntity.toCoreList(catalogPath), - PolarisEntityType.NAMESPACE, - PolarisEntitySubType.NULL_SUBTYPE) - .getEntities()); - return PolarisCatalogHelpers.nameAndIdToNamespaces(catalogPath, entities); + PolarisEntity.toNameAndIdList(listResult.getEntities()); + List namespaces = PolarisCatalogHelpers.nameAndIdToNamespaces(catalogPath, entities); + return listResult + .getPageToken() + .map(token -> new PolarisPage<>(token, namespaces)) + .orElseGet(() -> PolarisPage.fromData(namespaces)); } @Override @@ -797,12 +835,19 @@ public void close() throws IOException { @Override public List listViews(Namespace namespace) { + return listViews(namespace, ReadEverythingPageToken.get()).data; + } + + public PolarisPage listViews(Namespace namespace, PageToken pageToken) { if (!namespaceExists(namespace) && !namespace.isEmpty()) { throw new NoSuchNamespaceException( "Cannot list views for namespace. Namespace does not exist: %s", namespace); } + if (!paginationEnabled()) { + pageToken = ReadEverythingPageToken.get(); + } - return listTableLike(PolarisEntitySubType.VIEW, namespace); + return listTableLike(PolarisEntitySubType.VIEW, namespace, pageToken); } @Override @@ -1047,7 +1092,8 @@ private void validateNoLocationOverlap( callContext.getPolarisCallContext(), parentPath.stream().map(PolarisEntity::toCore).collect(Collectors.toList()), PolarisEntityType.NAMESPACE, - PolarisEntitySubType.ANY_SUBTYPE); + PolarisEntitySubType.ANY_SUBTYPE, + ReadEverythingPageToken.get()); if (!siblingNamespacesResult.isSuccess()) { throw new IllegalStateException( "Unable to resolve siblings entities to validate location - could not list namespaces"); @@ -1072,7 +1118,8 @@ private void validateNoLocationOverlap( .map(PolarisEntity::toCore) .collect(Collectors.toList()), PolarisEntityType.ICEBERG_TABLE_LIKE, - PolarisEntitySubType.ANY_SUBTYPE); + PolarisEntitySubType.ANY_SUBTYPE, + ReadEverythingPageToken.get()); if (!siblingTablesResult.isSuccess()) { throw new IllegalStateException( "Unable to resolve siblings entities to validate location - could not list tables"); @@ -2064,7 +2111,8 @@ private void createNonExistingNamespaces(Namespace namespace) { } } - private List listTableLike(PolarisEntitySubType subType, Namespace namespace) { + private PolarisPage listTableLike( + PolarisEntitySubType subType, Namespace namespace, PageToken pageToken) { PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); if (resolvedEntities == null) { // Illegal state because the namespace should've already been in the static resolution set. @@ -2073,16 +2121,23 @@ private List listTableLike(PolarisEntitySubType subType, Namesp } List catalogPath = resolvedEntities.getRawFullPath(); + ListEntitiesResult listResult = + getMetaStoreManager() + .listEntities( + getCurrentPolarisContext(), + PolarisEntity.toCoreList(catalogPath), + PolarisEntityType.ICEBERG_TABLE_LIKE, + subType, + pageToken); List entities = - PolarisEntity.toNameAndIdList( - getMetaStoreManager() - .listEntities( - getCurrentPolarisContext(), - PolarisEntity.toCoreList(catalogPath), - PolarisEntityType.ICEBERG_TABLE_LIKE, - subType) - .getEntities()); - return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities); + PolarisEntity.toNameAndIdList(listResult.getEntities()); + List identifiers = + PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities); + + return listResult + .getPageToken() + .map(token -> new PolarisPage<>(token, identifiers)) + .orElseGet(() -> PolarisPage.fromData(identifiers)); } /** diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index e7c0f13114..64da566dcb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -189,9 +189,7 @@ private IcebergCatalogHandlerWrapper newHandlerWrapper( } /** Build a {@link PageToken} from a string and page size. */ - private PageToken buildPageToken( - @Nullable String tokenString, - @Nullable Integer pageSize) { + private PageToken buildPageToken(@Nullable String tokenString, @Nullable Integer pageSize) { if (tokenString != null) { return callContext .getPolarisCallContext() @@ -200,7 +198,11 @@ private PageToken buildPageToken( .fromString(tokenString) .withPageSize(pageSize); } else { - return callContext.getPolarisCallContext().getMetaStore().pageTokenBuilder().fromLimit(pageSize); + return callContext + .getPolarisCallContext() + .getMetaStore() + .pageTokenBuilder() + .fromLimit(pageSize); } } @@ -231,7 +233,8 @@ public Response listNamespaces( securityContext, prefix, catalog -> - Response.ok(catalog.listNamespaces(namespaceOptional.orElse(Namespace.of()), token)).build()); + Response.ok(catalog.listNamespaces(namespaceOptional.orElse(Namespace.of()), token)) + .build()); } @Override diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java index 734007daf0..672408adb6 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerWrapper.java @@ -462,13 +462,13 @@ private void authorizeRenameTableLikeOperationOrThrow( initializeCatalog(); } - public ListNamespacesResponseWithPageToken listNamespaces(Namespace parent, PageToken pageToken) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES; authorizeBasicNamespaceOperationOrThrow(op, parent); if (baseCatalog instanceof IcebergCatalog polarisCatalog) { - return ListNamespacesResponseWithPageToken.fromPolarisPage(polarisCatalog.listNamespaces(parent, pageToken)); + return ListNamespacesResponseWithPageToken.fromPolarisPage( + polarisCatalog.listNamespaces(parent, pageToken)); } else { return ListNamespacesResponseWithPageToken.fromPolarisPage( PolarisPage.fromData( @@ -564,7 +564,8 @@ public ListTablesResponseWithPageToken listTables(Namespace namespace, PageToken authorizeBasicNamespaceOperationOrThrow(op, namespace); if (baseCatalog instanceof IcebergCatalog polarisCatalog) { - return ListTablesResponseWithPageToken.fromPolarisPage(polarisCatalog.listTables(namespace, pageToken)); + return ListTablesResponseWithPageToken.fromPolarisPage( + polarisCatalog.listTables(namespace, pageToken)); } else { return ListTablesResponseWithPageToken.fromPolarisPage( PolarisPage.fromData(CatalogHandlers.listTables(baseCatalog, namespace).identifiers())); @@ -1096,11 +1097,11 @@ public ListTablesResponseWithPageToken listViews(Namespace namespace, PageToken authorizeBasicNamespaceOperationOrThrow(op, namespace); if (baseCatalog instanceof IcebergCatalog polarisCatalog) { - return ListTablesResponseWithPageToken.fromPolarisPage(polarisCatalog.listViews(namespace, pageToken)); + return ListTablesResponseWithPageToken.fromPolarisPage( + polarisCatalog.listViews(namespace, pageToken)); } else { return ListTablesResponseWithPageToken.fromPolarisPage( - PolarisPage.fromData(CatalogHandlers.listTables(baseCatalog, namespace) - .identifiers())); + PolarisPage.fromData(CatalogHandlers.listTables(baseCatalog, namespace).identifiers())); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/types/ListTablesResponseWithPageToken.java b/service/common/src/main/java/org/apache/polaris/service/types/ListTablesResponseWithPageToken.java index e696f344bc..0c4dbaf009 100644 --- a/service/common/src/main/java/org/apache/polaris/service/types/ListTablesResponseWithPageToken.java +++ b/service/common/src/main/java/org/apache/polaris/service/types/ListTablesResponseWithPageToken.java @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package org.apache.polaris.service.types; import com.fasterxml.jackson.annotation.JsonProperty; @@ -34,40 +33,40 @@ * more results. */ public class ListTablesResponseWithPageToken extends ListTablesResponse { - private final PageToken pageToken; + private final PageToken pageToken; - private final List identifiers; + private final List identifiers; - public ListTablesResponseWithPageToken(PageToken pageToken, List identifiers) { - this.pageToken = pageToken; - this.identifiers = identifiers; - Preconditions.checkArgument(this.identifiers != null, "Invalid identifier list: null"); - } + public ListTablesResponseWithPageToken(PageToken pageToken, List identifiers) { + this.pageToken = pageToken; + this.identifiers = identifiers; + Preconditions.checkArgument(this.identifiers != null, "Invalid identifier list: null"); + } - public static ListTablesResponseWithPageToken fromPolarisPage( - PolarisPage polarisPage) { - return new ListTablesResponseWithPageToken(polarisPage.pageToken, polarisPage.data); - } + public static ListTablesResponseWithPageToken fromPolarisPage( + PolarisPage polarisPage) { + return new ListTablesResponseWithPageToken(polarisPage.pageToken, polarisPage.data); + } - @JsonProperty("next-page-token") - public String getPageToken() { - if (pageToken == null) { - return null; - } else { - return pageToken.toString(); - } + @JsonProperty("next-page-token") + public String getPageToken() { + if (pageToken == null) { + return null; + } else { + return pageToken.toString(); } + } - @Override - public List identifiers() { - return this.identifiers != null ? this.identifiers : List.of(); - } + @Override + public List identifiers() { + return this.identifiers != null ? this.identifiers : List.of(); + } - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("identifiers", this.identifiers) - .add("pageToken", this.pageToken) - .toString(); - } + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("identifiers", this.identifiers) + .add("pageToken", this.pageToken) + .toString(); + } } From 5c969d74e2768a47d28f15747fd725fdba6fa645 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 14:19:47 -0700 Subject: [PATCH 13/23] remaining issues are related to task loading --- .../BasePolarisMetaStoreManagerTest.java | 36 ++++++++++++++---- .../PolarisTestMetaStoreManager.java | 23 +++++++++--- .../quarkus/catalog/IcebergCatalogTest.java | 16 +++++++- .../task/TableCleanupTaskHandlerTest.java | 37 ++++++++++++++++--- .../service/catalog/io/FileIOFactoryTest.java | 5 ++- 5 files changed, 95 insertions(+), 22 deletions(-) diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java index 81f233825b..532bf4554c 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java @@ -43,6 +43,7 @@ import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.TaskEntity; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; import org.assertj.core.api.Assertions; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.BeforeEach; @@ -128,7 +129,8 @@ void testCreateEntities() { polarisTestMetaStoreManager.polarisCallContext, null, PolarisEntityType.TASK, - PolarisEntitySubType.NULL_SUBTYPE) + PolarisEntitySubType.NULL_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities(); Assertions.assertThat(listedEntities) .isNotNull() @@ -290,7 +292,9 @@ void testLoadTasks() { PolarisMetaStoreManager metaStoreManager = polarisTestMetaStoreManager.polarisMetaStoreManager; PolarisCallContext callCtx = polarisTestMetaStoreManager.polarisCallContext; List taskList = - metaStoreManager.loadTasks(callCtx, executorId, 5).getEntities(); + metaStoreManager + .loadTasks(callCtx, executorId, callCtx.getMetaStore().pageTokenBuilder().fromLimit(5)) + .getEntities(); Assertions.assertThat(taskList) .isNotNull() .isNotEmpty() @@ -310,7 +314,9 @@ void testLoadTasks() { // grab a second round of tasks. Assert that none of the original 5 are in the list List newTaskList = - metaStoreManager.loadTasks(callCtx, executorId, 5).getEntities(); + metaStoreManager + .loadTasks(callCtx, executorId, callCtx.getMetaStore().pageTokenBuilder().fromLimit(5)) + .getEntities(); Assertions.assertThat(newTaskList) .isNotNull() .isNotEmpty() @@ -324,7 +330,9 @@ void testLoadTasks() { // only 10 tasks are unassigned. Requesting 20, we should only receive those 10 List lastTen = - metaStoreManager.loadTasks(callCtx, executorId, 20).getEntities(); + metaStoreManager + .loadTasks(callCtx, executorId, callCtx.getMetaStore().pageTokenBuilder().fromLimit(20)) + .getEntities(); Assertions.assertThat(lastTen) .isNotNull() @@ -338,7 +346,9 @@ void testLoadTasks() { .collect(Collectors.toSet()); List emtpyList = - metaStoreManager.loadTasks(callCtx, executorId, 20).getEntities(); + metaStoreManager + .loadTasks(callCtx, executorId, callCtx.getMetaStore().pageTokenBuilder().fromLimit(20)) + .getEntities(); Assertions.assertThat(emtpyList).isNotNull().isEmpty(); @@ -346,7 +356,9 @@ void testLoadTasks() { // all the tasks are unassigned. Fetch them all List allTasks = - metaStoreManager.loadTasks(callCtx, executorId, 20).getEntities(); + metaStoreManager + .loadTasks(callCtx, executorId, callCtx.getMetaStore().pageTokenBuilder().fromLimit(20)) + .getEntities(); Assertions.assertThat(allTasks) .isNotNull() @@ -361,7 +373,9 @@ void testLoadTasks() { timeSource.add(Duration.ofMinutes(10)); List finalList = - metaStoreManager.loadTasks(callCtx, executorId, 20).getEntities(); + metaStoreManager + .loadTasks(callCtx, executorId, callCtx.getMetaStore().pageTokenBuilder().fromLimit(20)) + .getEntities(); Assertions.assertThat(finalList).isNotNull().isEmpty(); } @@ -389,7 +403,13 @@ void testLoadTasksInParallel() throws Exception { do { retry = false; try { - taskList = metaStoreManager.loadTasks(callCtx, executorId, 5).getEntities(); + taskList = + metaStoreManager + .loadTasks( + callCtx, + executorId, + callCtx.getMetaStore().pageTokenBuilder().fromLimit(5)) + .getEntities(); taskList.stream().map(PolarisBaseEntity::getName).forEach(taskNames::add); } catch (RetryOnConcurrencyException e) { retry = true; diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index b8470dee94..b437947d6e 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -49,6 +49,7 @@ import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.policy.PredefinedPolicyTypes; @@ -733,7 +734,8 @@ void dropEntity(List catalogPath, PolarisBaseEntity entityToD this.polarisCallContext, path, PolarisEntityType.NAMESPACE, - PolarisEntitySubType.NULL_SUBTYPE) + PolarisEntitySubType.NULL_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities(); Assertions.assertThat(children).isNotNull(); if (children.isEmpty() && entity.getType() == PolarisEntityType.NAMESPACE) { @@ -743,7 +745,8 @@ void dropEntity(List catalogPath, PolarisBaseEntity entityToD this.polarisCallContext, path, PolarisEntityType.ICEBERG_TABLE_LIKE, - PolarisEntitySubType.ANY_SUBTYPE) + PolarisEntitySubType.ANY_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities(); Assertions.assertThat(children).isNotNull(); } else if (children.isEmpty()) { @@ -753,7 +756,8 @@ void dropEntity(List catalogPath, PolarisBaseEntity entityToD this.polarisCallContext, path, PolarisEntityType.CATALOG_ROLE, - PolarisEntitySubType.ANY_SUBTYPE) + PolarisEntitySubType.ANY_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities(); Assertions.assertThat(children).isNotNull(); // if only one left, it can be dropped. @@ -1286,7 +1290,12 @@ private void validateListReturn( // list the entities under the specified path List result = polarisMetaStoreManager - .listEntities(this.polarisCallContext, path, entityType, entitySubType) + .listEntities( + this.polarisCallContext, + path, + entityType, + entitySubType, + ReadEverythingPageToken.get()) .getEntities(); Assertions.assertThat(result).isNotNull(); @@ -1603,7 +1612,8 @@ void validateBootstrap() { this.polarisCallContext, null, PolarisEntityType.PRINCIPAL, - PolarisEntitySubType.NULL_SUBTYPE) + PolarisEntitySubType.NULL_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities(); // ensure not null, one element only @@ -1629,7 +1639,8 @@ void validateBootstrap() { this.polarisCallContext, null, PolarisEntityType.PRINCIPAL_ROLE, - PolarisEntitySubType.NULL_SUBTYPE) + PolarisEntitySubType.NULL_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities(); // ensure not null, one element only diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 3dafaf109a..96c6f079a7 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -1423,7 +1423,12 @@ public void testDropTableWithPurge() { .as("Table should not exist after drop") .rejects(TABLE); List tasks = - metaStoreManager.loadTasks(polarisContext, "testExecutor", 1).getEntities(); + metaStoreManager + .loadTasks( + polarisContext, + "testExecutor", + callContext.getPolarisCallContext().getMetaStore().pageTokenBuilder().fromLimit(1)) + .getEntities(); Assertions.assertThat(tasks).hasSize(1); TaskEntity taskEntity = TaskEntity.of(tasks.get(0)); EnumMap credentials = @@ -1625,7 +1630,14 @@ public void testFileIOWrapper() { TaskEntity taskEntity = TaskEntity.of( metaStoreManager - .loadTasks(callContext.getPolarisCallContext(), "testExecutor", 1) + .loadTasks( + callContext.getPolarisCallContext(), + "testExecutor", + callContext + .getPolarisCallContext() + .getMetaStore() + .pageTokenBuilder() + .fromLimit(1)) .getEntities() .getFirst()); Map properties = taskEntity.getInternalPropertiesAsMap(); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java index d723f6caf0..88f39a50ca 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java @@ -141,7 +141,14 @@ public void testTableCleanup() throws IOException { assertThat( metaStoreManagerFactory .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "test", 2) + .loadTasks( + callContext.getPolarisCallContext(), + "test", + callContext + .getPolarisCallContext() + .getMetaStore() + .pageTokenBuilder() + .fromLimit(2)) .getEntities()) .hasSize(2) .satisfiesExactlyInAnyOrder( @@ -221,7 +228,14 @@ public void close() { assertThat( metaStoreManagerFactory .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "test", 5) + .loadTasks( + callContext.getPolarisCallContext(), + "test", + callContext + .getPolarisCallContext() + .getMetaStore() + .pageTokenBuilder() + .fromLimit(5)) .getEntities()) .hasSize(2); } @@ -282,7 +296,14 @@ public void close() { assertThat( metaStoreManagerFactory .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "test", 5) + .loadTasks( + callContext.getPolarisCallContext(), + "test", + callContext + .getPolarisCallContext() + .getMetaStore() + .pageTokenBuilder() + .fromLimit(5)) .getEntities()) .hasSize(4) .satisfiesExactly( @@ -402,7 +423,10 @@ public void testTableCleanupMultipleSnapshots() throws IOException { List entities = metaStoreManagerFactory .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "test", 5) + .loadTasks( + callContext.getPolarisCallContext(), + "test", + callContext.getPolarisCallContext().getMetaStore().pageTokenBuilder().fromLimit(5)) .getEntities(); List manifestCleanupTasks = @@ -561,7 +585,10 @@ public void testTableCleanupMultipleMetadata() throws IOException { List entities = metaStoreManagerFactory .getOrCreateMetaStoreManager(callContext.getRealmContext()) - .loadTasks(callContext.getPolarisCallContext(), "test", 6) + .loadTasks( + callContext.getPolarisCallContext(), + "test", + callContext.getPolarisCallContext().getMetaStore().pageTokenBuilder().fromLimit(6)) .getEntities(); List manifestCleanupTasks = diff --git a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java index 9a070273b8..2afceead8e 100644 --- a/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java +++ b/service/common/src/test/java/org/apache/polaris/service/catalog/io/FileIOFactoryTest.java @@ -184,7 +184,10 @@ public void testLoadFileIOForCleanupTask() { testServices .metaStoreManagerFactory() .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "testExecutor", 1) + .loadTasks( + callContext.getPolarisCallContext(), + "testExecutor", + callContext.getPolarisCallContext().getMetaStore().pageTokenBuilder().fromLimit(1)) .getEntities(); Assertions.assertThat(tasks).hasSize(1); TaskEntity taskEntity = TaskEntity.of(tasks.get(0)); From e67b0e984af88bd8c6beef928f92fc554f5d3201 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 24 Mar 2025 14:32:18 -0700 Subject: [PATCH 14/23] re-add tests --- .../TreeMapTransactionalPersistenceImpl.java | 2 +- .../quarkus/catalog/IcebergCatalogTest.java | 131 +++++++++++++++++- 2 files changed, 131 insertions(+), 2 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 44ef9422bb..347ff26fb7 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -334,7 +334,7 @@ public List lookupEntityActiveBatchInCurrentTxn( entity.getName(), entity.getTypeCode(), entity.getSubTypeCode()), - ReadEverythingPageToken.get()); + pageToken); } @Override diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 96c6f079a7..d36077d61f 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -98,6 +98,7 @@ import org.apache.polaris.core.persistence.dao.entity.BaseResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult; +import org.apache.polaris.core.persistence.pagination.PolarisPage; import org.apache.polaris.core.persistence.transactional.TransactionalPersistence; import org.apache.polaris.core.storage.PolarisCredentialProperty; import org.apache.polaris.core.storage.PolarisStorageActions; @@ -154,7 +155,9 @@ public Map getConfigOverrides() { "polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", "true", "polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", - "[\"FILE\"]"); + "[\"FILE\"]", + "polaris.features.defaults.\"LIST_PAGINATION_ENABLED\"", + "true"); } } @@ -1783,4 +1786,130 @@ public void testConcurrencyConflictUpdateTableDuringFinalTransaction() { private static InMemoryFileIO getInMemoryIo(IcebergCatalog catalog) { return (InMemoryFileIO) ((ExceptionMappingFileIO) catalog.getIo()).getInnerIo(); } + + @Test + public void testPaginatedListTables() { + if (this.requiresNamespaceCreate()) { + ((SupportsNamespaces) catalog).createNamespace(NS); + } + + for (int i = 0; i < 5; i++) { + catalog.buildTable(TableIdentifier.of(NS, "pagination_table_" + i), SCHEMA).create(); + } + + try { + // List without pagination + Assertions.assertThat(catalog.listTables(NS)).isNotNull().hasSize(5); + + // List with a limit: + PolarisPage firstListResult = + catalog.listTables(NS, polarisContext.getMetaStore().pageTokenBuilder().fromLimit(2)); + Assertions.assertThat(firstListResult.data.size()).isEqualTo(2); + Assertions.assertThat(firstListResult.pageToken.toString()).isNotNull().isNotEmpty(); + + // List using the previously obtained token: + PolarisPage secondListResult = catalog.listTables(NS, firstListResult.pageToken); + Assertions.assertThat(secondListResult.data.size()).isEqualTo(2); + Assertions.assertThat(secondListResult.pageToken.toString()).isNotNull().isNotEmpty(); + + // List using the final token: + PolarisPage finalListResult = catalog.listTables(NS, secondListResult.pageToken); + Assertions.assertThat(finalListResult.data.size()).isEqualTo(1); + Assertions.assertThat(finalListResult.pageToken).isNull(); + } finally { + for (int i = 0; i < 5; i++) { + catalog.dropTable(TableIdentifier.of(NS, "pagination_table_" + i)); + } + } + } + + @Test + public void testPaginatedListViews() { + if (this.requiresNamespaceCreate()) { + ((SupportsNamespaces) catalog).createNamespace(NS); + } + + for (int i = 0; i < 5; i++) { + catalog + .buildView(TableIdentifier.of(NS, "pagination_view_" + i)) + .withQuery("a_" + i, "SELECT 1 id") + .withSchema(SCHEMA) + .withDefaultNamespace(NS) + .create(); + } + + try { + // List without pagination + Assertions.assertThat(catalog.listViews(NS)).isNotNull().hasSize(5); + + // List with a limit: + PolarisPage firstListResult = + catalog.listViews(NS, polarisContext.getMetaStore().pageTokenBuilder().fromLimit(2)); + Assertions.assertThat(firstListResult.data.size()).isEqualTo(2); + Assertions.assertThat(firstListResult.pageToken.toString()).isNotNull().isNotEmpty(); + + // List using the previously obtained token: + PolarisPage secondListResult = catalog.listViews(NS, firstListResult.pageToken); + Assertions.assertThat(secondListResult.data.size()).isEqualTo(2); + Assertions.assertThat(secondListResult.pageToken.toString()).isNotNull().isNotEmpty(); + + // List using the final token: + PolarisPage finalListResult = catalog.listViews(NS, secondListResult.pageToken); + Assertions.assertThat(finalListResult.data.size()).isEqualTo(1); + Assertions.assertThat(finalListResult.pageToken).isNull(); + } finally { + for (int i = 0; i < 5; i++) { + catalog.dropTable(TableIdentifier.of(NS, "pagination_view_" + i)); + } + } + } + + @Test + public void testPaginatedListNamespaces() { + for (int i = 0; i < 5; i++) { + catalog.createNamespace(Namespace.of("pagination_namespace_" + i)); + } + + try { + // List without pagination + Assertions.assertThat(catalog.listNamespaces()).isNotNull().hasSize(5); + + // List with a limit: + PolarisPage firstListResult = + catalog.listNamespaces(polarisContext.getMetaStore().pageTokenBuilder().fromLimit(2)); + Assertions.assertThat(firstListResult.data.size()).isEqualTo(2); + Assertions.assertThat(firstListResult.pageToken.toString()).isNotNull().isNotEmpty(); + + // List using the previously obtained token: + PolarisPage secondListResult = catalog.listNamespaces(firstListResult.pageToken); + Assertions.assertThat(secondListResult.data.size()).isEqualTo(2); + Assertions.assertThat(secondListResult.pageToken.toString()).isNotNull().isNotEmpty(); + + // List using the final token: + PolarisPage finalListResult = catalog.listNamespaces(secondListResult.pageToken); + Assertions.assertThat(finalListResult.data.size()).isEqualTo(1); + Assertions.assertThat(finalListResult.pageToken).isNull(); + + // List with page size matching the amount of data + PolarisPage firstExactListResult = + catalog.listNamespaces(polarisContext.getMetaStore().pageTokenBuilder().fromLimit(5)); + Assertions.assertThat(firstExactListResult.data.size()).isEqualTo(5); + Assertions.assertThat(firstExactListResult.pageToken.toString()).isNotNull().isNotEmpty(); + + // Again list with matching page size + PolarisPage secondExactListResult = catalog.listNamespaces(firstExactListResult.pageToken); + Assertions.assertThat(secondExactListResult.data).isEmpty(); + Assertions.assertThat(secondExactListResult.pageToken).isNull(); + + // List with huge page size: + PolarisPage bigListResult = + catalog.listNamespaces(polarisContext.getMetaStore().pageTokenBuilder().fromLimit(9999)); + Assertions.assertThat(bigListResult.data.size()).isEqualTo(5); + Assertions.assertThat(bigListResult.pageToken).isNull(); + } finally { + for (int i = 0; i < 5; i++) { + catalog.dropNamespace(Namespace.of("pagination_namespace_" + i)); + } + } + } } From 663dafb0cf060466f1e33022184bc40263402364 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 27 Mar 2025 01:11:57 -0700 Subject: [PATCH 15/23] debugging --- .../TreeMapTransactionalPersistenceImpl.java | 4 ++-- .../catalog/iceberg/IcebergCatalog.java | 21 +++---------------- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 347ff26fb7..05821795a3 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -362,8 +362,8 @@ public List lookupEntityActiveBatchInCurrentTxn( if (pageToken instanceof OffsetPageToken) { partialResults = partialResults - .sorted(Comparator.comparingLong(PolarisEntityCore::getId)) - .skip(((OffsetPageToken) pageToken).offset) + //.sorted(Comparator.comparingLong(PolarisEntityCore::getId)) + //.skip(((OffsetPageToken) pageToken).offset) .limit(pageToken.pageSize); } 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 d692f462ef..6bd73090e5 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 @@ -1239,20 +1239,6 @@ private class BasePolarisTableOperations extends BaseMetastoreTableOperations { this.tableFileIO = defaultFileIO; } - protected PolarisResolvedPathWrapper getTablePath(TableIdentifier tableIdentifier) { - PolarisResolvedPathWrapper icebergTableResult = - resolvedEntityView.getPassthroughResolvedPath( - tableIdentifier, - PolarisEntityType.ICEBERG_TABLE_LIKE, - PolarisEntitySubType.ANY_SUBTYPE); - if (icebergTableResult == null) { - return resolvedEntityView.getPassthroughResolvedPath( - tableIdentifier, PolarisEntityType.GENERIC_TABLE, PolarisEntitySubType.ANY_SUBTYPE); - } else { - return icebergTableResult; - } - } - @Override public void doRefresh() { LOGGER.debug("doRefresh for tableIdentifier {}", tableIdentifier); @@ -1398,7 +1384,9 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { // concurrent // modification between our checking of unchanged metadataLocation here and actual // persistence-layer commit). - PolarisResolvedPathWrapper resolvedEntities = getTablePath(tableIdentifier); + PolarisResolvedPathWrapper resolvedEntities = + resolvedEntityView.getPassthroughResolvedPath( + tableIdentifier, PolarisEntityType.ICEBERG_TABLE_LIKE, PolarisEntitySubType.TABLE); IcebergTableLikeEntity entity = IcebergTableLikeEntity.of( resolvedEntities == null ? null : resolvedEntities.getRawLeafEntity()); @@ -1421,9 +1409,6 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { .setMetadataLocation(newLocation) .build(); } - if (entity.getType() == PolarisEntityType.GENERIC_TABLE) { - throw new AlreadyExistsException("Table already exists: %s", tableName()); - } if (!Objects.equal(existingLocation, oldLocation)) { if (null == base) { throw new AlreadyExistsException("Table already exists: %s", tableName()); From 133b658731986664fead84c4472bc655a01da178 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 27 Mar 2025 01:24:51 -0700 Subject: [PATCH 16/23] fix failing tests --- .../TreeMapTransactionalPersistenceImpl.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 05821795a3..42c020c4f0 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -357,13 +357,17 @@ public List lookupEntityActiveBatchInCurrentTxn( .readRange( this.store.buildPrefixKeyComposite(catalogId, parentId, entityType.getCode())) .stream() + .map( + nameRecord -> + this.lookupEntityInCurrentTxn( + callCtx, catalogId, nameRecord.getId(), entityType.getCode())) .filter(entityFilter); if (pageToken instanceof OffsetPageToken) { partialResults = partialResults - //.sorted(Comparator.comparingLong(PolarisEntityCore::getId)) - //.skip(((OffsetPageToken) pageToken).offset) + .sorted(Comparator.comparingLong(PolarisEntityCore::getId)) + .skip(((OffsetPageToken) pageToken).offset) .limit(pageToken.pageSize); } From 98661beb1b7d10f3c8095dc2a51dce59c76ffee1 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 27 Mar 2025 01:25:37 -0700 Subject: [PATCH 17/23] fix another test --- .../service/quarkus/task/TableCleanupTaskHandlerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java index 3d036163ac..21d27decc9 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/task/TableCleanupTaskHandlerTest.java @@ -306,7 +306,7 @@ public void close() { .fromLimit(5)) .getEntities()) .hasSize(4) - .satisfiesExactly( + .satisfiesExactlyInAnyOrder( taskEntity -> assertThat(taskEntity) .returns(PolarisEntityType.TASK.getCode(), PolarisBaseEntity::getTypeCode) From 0798b63bf3d0934f7baa966b1d9cd7aa0621df59 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 31 Mar 2025 20:57:02 -0700 Subject: [PATCH 18/23] changes per review --- .../persistence/impl/eclipselink/PolarisEclipseLinkStore.java | 2 +- .../org/apache/polaris/core/config/FeatureConfiguration.java | 3 ++- .../service/quarkus/catalog/GenericTableCatalogTest.java | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java index 914f55b7b4..a91d87d81a 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java @@ -285,7 +285,7 @@ List lookupFullEntitiesActive( long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - PageToken pageToken) { + @Nonnull PageToken pageToken) { diagnosticServices.check(session != null, "session_is_null"); diagnosticServices.check( (pageToken instanceof EntityIdPageToken || pageToken instanceof ReadEverythingPageToken), diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java index 75ebbeab29..dba0ad0363 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java @@ -188,7 +188,8 @@ protected FeatureConfiguration( PolarisConfiguration.builder() .key("LIST_PAGINATION_ENABLED") .catalogConfig("list-pagination.enabled") - .description("If set to true, pagination for APIs like listTables is enabled") + .description("If set to true, pagination for APIs like listTables is enabled. The APIs that" + + " currently support pagination are listTables, listViews, and listNamespaces.") .defaultValue(false) .buildFeatureConfiguration(); } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java index e8115cc4cf..9fcdabc24e 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java @@ -534,6 +534,7 @@ public void testDropIcebergTable() { Assertions.assertThatCode( () -> genericTableCatalog.dropGenericTable(TableIdentifier.of("ns", "t1"))) .hasMessageContaining("Generic table does not exist: ns.t1"); + Assertions.assertThatCode(() -> icebergCatalog.dropTable(TableIdentifier.of("ns", "t1"))) .doesNotThrowAnyException(); } From 241e7ed7c0a837ffae0aac8f0e3241f8420dda92 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 31 Mar 2025 20:57:11 -0700 Subject: [PATCH 19/23] autolint --- .../org/apache/polaris/core/config/FeatureConfiguration.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java index dba0ad0363..2d8a91a407 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java @@ -188,8 +188,9 @@ protected FeatureConfiguration( PolarisConfiguration.builder() .key("LIST_PAGINATION_ENABLED") .catalogConfig("list-pagination.enabled") - .description("If set to true, pagination for APIs like listTables is enabled. The APIs that" + - " currently support pagination are listTables, listViews, and listNamespaces.") + .description( + "If set to true, pagination for APIs like listTables is enabled. The APIs that" + + " currently support pagination are listTables, listViews, and listNamespaces.") .defaultValue(false) .buildFeatureConfiguration(); } From 0ad4ced22c0deec215de620519ca8f1a809118e6 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 1 Apr 2025 10:03:44 -0700 Subject: [PATCH 20/23] some fixes --- .../polaris/core/persistence/dao/entity/ListEntitiesResult.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ListEntitiesResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ListEntitiesResult.java index f43985ec82..9d66d5ae51 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ListEntitiesResult.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ListEntitiesResult.java @@ -62,7 +62,7 @@ public ListEntitiesResult( * @param entities list of entities being returned, implies success */ public ListEntitiesResult( - @Nonnull List entities, @Nullable Optional pageTokenOpt) { + @Nonnull List entities, @Nonnull Optional pageTokenOpt) { super(ReturnStatus.SUCCESS); this.entities = entities; this.pageTokenOpt = pageTokenOpt; From dc864410bf4add2701cd33ddd1abe155c7a15d5b Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 2 Apr 2025 21:47:45 -0700 Subject: [PATCH 21/23] stable --- .../impl/eclipselink/PolarisEclipseLinkStore.java | 2 +- .../core/persistence/PolarisTestMetaStoreManager.java | 3 --- .../polaris/service/quarkus/catalog/IcebergCatalogTest.java | 1 + .../service/catalog/iceberg/IcebergCatalogHandler.java | 6 ++---- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java index 09dc2b9908..2cdf9fe0ed 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java @@ -37,8 +37,8 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; -import org.apache.polaris.jpa.models.EntityIdPageToken; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; +import org.apache.polaris.jpa.models.EntityIdPageToken; import org.apache.polaris.jpa.models.ModelEntity; import org.apache.polaris.jpa.models.ModelEntityActive; import org.apache.polaris.jpa.models.ModelEntityChangeTracking; diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index 665b812933..54cb3e0394 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -51,11 +51,8 @@ import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult; import org.apache.polaris.core.persistence.dao.entity.PolicyAttachmentResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; -<<<<<<< HEAD import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; -======= import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; ->>>>>>> b50e594ed25297a112e721c8fcf4cd878489c664 import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.policy.PredefinedPolicyTypes; diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index e680b77c09..f792c395b7 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -62,6 +62,7 @@ import org.apache.iceberg.UpdateSchema; import org.apache.iceberg.catalog.CatalogTests; import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.BadRequestException; diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index efdb06548a..ce65111839 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -83,16 +83,14 @@ import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.pagination.PolarisPage; -import org.apache.polaris.core.persistence.resolver.ResolverPath; -import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.SupportsNotifications; import org.apache.polaris.service.catalog.common.CatalogHandler; import org.apache.polaris.service.context.CallContextCatalogFactory; -import org.apache.polaris.service.types.ListNamespacesResponseWithPageToken; -import org.apache.polaris.service.types.ListTablesResponseWithPageToken; import org.apache.polaris.service.http.IcebergHttpUtil; import org.apache.polaris.service.http.IfNoneMatch; +import org.apache.polaris.service.types.ListNamespacesResponseWithPageToken; +import org.apache.polaris.service.types.ListTablesResponseWithPageToken; import org.apache.polaris.service.types.NotificationRequest; import org.slf4j.Logger; import org.slf4j.LoggerFactory; From 0fcec2f5c573ac050755c93ecbc1f25f6b5c191c Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 28 Apr 2025 13:40:11 -0700 Subject: [PATCH 22/23] updates for new persistence --- .../polaris/jpa/models/EntityIdPageToken.java | 10 +- .../relational-jdbc/build.gradle.kts | 1 + .../relational/jdbc/DatasourceOperations.java | 11 +- .../jdbc/JdbcBasePersistenceImpl.java | 109 +++++++++++++----- .../jdbc/JdbcMetaStoreManagerFactory.java | 3 +- .../relational/jdbc/QueryGenerator.java | 19 +++ ...anagerWithJdbcBasePersistenceImplTest.java | 7 +- .../jdbc/DatasourceOperationsTest.java | 3 +- .../pagination/ReadEverythingPageToken.java | 2 +- .../PolarisTestMetaStoreManager.java | 3 +- .../catalog/iceberg/IcebergCatalog.java | 8 -- .../iceberg/IcebergCatalogHandler.java | 8 +- 12 files changed, 129 insertions(+), 55 deletions(-) diff --git a/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/EntityIdPageToken.java b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/EntityIdPageToken.java index 1573082977..7035420fd6 100644 --- a/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/EntityIdPageToken.java +++ b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/EntityIdPageToken.java @@ -89,16 +89,16 @@ public PageToken updated(List newData) { if (newData == null || newData.size() < this.pageSize) { return DONE; } else { - var head = newData.get(0); + var head = newData.getFirst(); if (head instanceof ModelEntity) { - return new EntityIdPageToken( - ((ModelEntity) newData.get(newData.size() - 1)).getId(), this.pageSize); + return new EntityIdPageToken(((ModelEntity) newData.getLast()).getId(), this.pageSize); } else if (head instanceof PolarisBaseEntity) { + // Assumed to be sorted with the greatest entity ID last return new EntityIdPageToken( - ((PolarisBaseEntity) newData.get(newData.size() - 1)).getId(), this.pageSize); + ((PolarisBaseEntity) newData.getLast()).getId(), this.pageSize); } else { throw new IllegalArgumentException( - "Cannot build a page token from: " + newData.get(0).getClass().getSimpleName()); + "Cannot build a page token from: " + newData.getFirst().getClass().getSimpleName()); } } } diff --git a/extension/persistence/relational-jdbc/build.gradle.kts b/extension/persistence/relational-jdbc/build.gradle.kts index 82f67c8a5a..59f0454f3d 100644 --- a/extension/persistence/relational-jdbc/build.gradle.kts +++ b/extension/persistence/relational-jdbc/build.gradle.kts @@ -24,6 +24,7 @@ plugins { dependencies { implementation(project(":polaris-core")) + implementation(project(":polaris-jpa-model")) implementation(libs.slf4j.api) implementation(libs.guava) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java index 6fec8e67fe..e29bf02cb8 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java @@ -34,6 +34,7 @@ import java.util.function.Function; import java.util.function.Predicate; import javax.sql.DataSource; +import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; public class DatasourceOperations { @@ -95,8 +96,8 @@ public void executeScript(String scriptFilePath) throws SQLException { * @param query : Query to executed * @param entityClass : Class of the entity being selected * @param transformer : Transformation of entity class to Result class - * @param entityFilter : Filter to applied on the Result class - * @param limit : Limit to to enforced. + * @param entityFilter : Client-side filter to applied on the Result class + * @param pageToken : Page token to be enforced. * @return List of Result class objects * @param : Entity class * @param : Result class @@ -107,13 +108,13 @@ public List executeSelect( @Nonnull Class entityClass, @Nonnull Function transformer, Predicate entityFilter, - int limit) + PageToken pageToken) throws SQLException { try (Connection connection = borrowConnection(); Statement statement = connection.createStatement(); ResultSet resultSet = statement.executeQuery(query)) { - List resultList = new ArrayList<>(); - while (resultSet.next() && resultList.size() < limit) { + List resultList = new ArrayList<>(PageToken.DEFAULT_PAGE_SIZE); + while (resultSet.next() && resultList.size() < pageToken.pageSize) { Converter object = (Converter) entityClass.getDeclaredConstructor().newInstance(); // Create a new instance diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 5ffce813f9..7fcd1dfc49 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -21,6 +21,7 @@ import static org.apache.polaris.extension.persistence.relational.jdbc.QueryGenerator.*; import com.google.common.base.Preconditions; +import com.google.common.base.Predicates; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.sql.SQLException; @@ -34,6 +35,7 @@ import java.util.function.Predicate; import java.util.stream.Collectors; import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.PolarisDiagnostics; import org.apache.polaris.core.entity.EntityNameLookupRecord; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisChangeTrackingVersions; @@ -49,6 +51,9 @@ import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException; import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PolarisPage; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; @@ -58,6 +63,7 @@ import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelGrantRecord; import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelPolicyMappingRecord; import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelPrincipalAuthenticationData; +import org.apache.polaris.jpa.models.EntityIdPageToken; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -69,16 +75,19 @@ public class JdbcBasePersistenceImpl implements BasePersistence, IntegrationPers private final PrincipalSecretsGenerator secretsGenerator; private final PolarisStorageIntegrationProvider storageIntegrationProvider; private final String realmId; + private final PolarisDiagnostics polarisDiagnostics; public JdbcBasePersistenceImpl( DatasourceOperations databaseOperations, PrincipalSecretsGenerator secretsGenerator, PolarisStorageIntegrationProvider storageIntegrationProvider, - String realmId) { + String realmId, + PolarisDiagnostics polarisDiagnostics) { this.datasourceOperations = databaseOperations; this.secretsGenerator = secretsGenerator; this.storageIntegrationProvider = storageIntegrationProvider; this.realmId = realmId; + this.polarisDiagnostics = polarisDiagnostics; } @Override @@ -303,7 +312,7 @@ private PolarisBaseEntity getPolarisBaseEntity(String query) { try { List results = datasourceOperations.executeSelect( - query, ModelEntity.class, ModelEntity::toEntity, null, Integer.MAX_VALUE); + query, ModelEntity.class, ModelEntity::toEntity, null, ReadEverythingPageToken.get()); if (results.isEmpty()) { return null; } else if (results.size() > 1) { @@ -328,7 +337,7 @@ public List lookupEntities( String query = generateSelectQueryWithEntityIds(realmId, entityIds); try { return datasourceOperations.executeSelect( - query, ModelEntity.class, ModelEntity::toEntity, null, Integer.MAX_VALUE); + query, ModelEntity.class, ModelEntity::toEntity, null, ReadEverythingPageToken.get()); } catch (SQLException e) { throw new RuntimeException( String.format("Failed to retrieve polaris entities due to %s", e.getMessage()), e); @@ -359,49 +368,55 @@ public List lookupEntityVersions( @Nonnull @Override - public List listEntities( + public PolarisPage listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, - @Nonnull PolarisEntityType entityType) { + @Nonnull PolarisEntityType entityType, + @Nonnull PageToken pageToken) { return listEntities( callCtx, catalogId, parentId, entityType, - Integer.MAX_VALUE, entity -> true, - EntityNameLookupRecord::new); + EntityNameLookupRecord::new, + pageToken); } @Nonnull @Override - public List listEntities( + public PolarisPage listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull Predicate entityFilter) { + @Nonnull Predicate entityFilter, + @Nonnull PageToken pageToken) { return listEntities( callCtx, catalogId, parentId, entityType, - Integer.MAX_VALUE, entityFilter, - EntityNameLookupRecord::new); + EntityNameLookupRecord::new, + pageToken); } @Nonnull @Override - public List listEntities( + public PolarisPage listEntities( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, PolarisEntityType entityType, - int limit, @Nonnull Predicate entityFilter, - @Nonnull Function transformer) { + @Nonnull Function transformer, + PageToken pageToken) { + polarisDiagnostics.check( + (pageToken instanceof EntityIdPageToken || pageToken instanceof ReadEverythingPageToken), + "unexpected_page_token"); + Map params = Map.of( "catalog_id", @@ -416,17 +431,53 @@ public List listEntities( // Limit can't be pushed down, due to client side filtering // absence of transaction. String query = QueryGenerator.generateSelectQuery(ModelEntity.class, params); + if (!(pageToken instanceof ReadEverythingPageToken)) { + query = QueryGenerator.updateQueryWithPageToken(query, pageToken); + } + final List data; try { - List results = - datasourceOperations.executeSelect( - query, ModelEntity.class, ModelEntity::toEntity, entityFilter, limit); - return results == null - ? Collections.emptyList() - : results.stream().filter(entityFilter).map(transformer).collect(Collectors.toList()); + if (entityFilter.equals(Predicates.alwaysTrue())) { + // In this case, we can push the filter down into the query + data = + datasourceOperations + .executeSelect( + query, ModelEntity.class, ModelEntity::toEntity, entityFilter, pageToken) + .stream() + .map(transformer) + .toList(); + } else { + // In this case, we cannot push the filter down into the query. We must therefore remove + // the page size limit from the PageToken and filter on the client side. + // TODO Implement a generic predicate that can be pushed down into different metastores + PageToken unlimitedPageSizeToken = pageToken.withPageSize(Integer.MAX_VALUE); + List rawData = + datasourceOperations.executeSelect( + query, + ModelEntity.class, + ModelEntity::toEntity, + entityFilter, + unlimitedPageSizeToken); + if (pageToken.pageSize < Integer.MAX_VALUE && rawData.size() > pageToken.pageSize) { + LOGGER.info( + "A page token could not be respected due to a predicate. " + + "{} records were read but the client was asked to return {}.", + rawData.size(), + pageToken.pageSize); + } + + data = + rawData.stream() + .filter(entityFilter) + .limit(pageToken.pageSize) + .map(transformer) + .collect(Collectors.toList()); + } } catch (SQLException e) { throw new RuntimeException( String.format("Failed to retrieve polaris entities due to %s", e.getMessage()), e); } + + return pageToken.buildNextPage(data); } @Override @@ -470,7 +521,7 @@ public PolarisGrantRecord lookupGrantRecord( ModelGrantRecord.class, ModelGrantRecord::toGrantRecord, null, - Integer.MAX_VALUE); + ReadEverythingPageToken.get()); if (results.size() > 1) { throw new IllegalStateException( String.format( @@ -505,7 +556,7 @@ public List loadAllGrantRecordsOnSecurable( ModelGrantRecord.class, ModelGrantRecord::toGrantRecord, null, - Integer.MAX_VALUE); + ReadEverythingPageToken.get()); return results == null ? Collections.emptyList() : results; } catch (SQLException e) { throw new RuntimeException( @@ -531,7 +582,7 @@ public List loadAllGrantRecordsOnGrantee( ModelGrantRecord.class, ModelGrantRecord::toGrantRecord, null, - Integer.MAX_VALUE); + ReadEverythingPageToken.get()); return results == null ? Collections.emptyList() : results; } catch (SQLException e) { throw new RuntimeException( @@ -559,7 +610,7 @@ public boolean hasChildren( try { List results = datasourceOperations.executeSelect( - query, ModelEntity.class, Function.identity(), null, Integer.MAX_VALUE); + query, ModelEntity.class, Function.identity(), null, ReadEverythingPageToken.get()); return results != null && !results.isEmpty(); } catch (SQLException e) { throw new RuntimeException( @@ -582,7 +633,7 @@ public PolarisPrincipalSecrets loadPrincipalSecrets( ModelPrincipalAuthenticationData.class, ModelPrincipalAuthenticationData::toPrincipalAuthenticationData, null, - Integer.MAX_VALUE); + ReadEverythingPageToken.get()); return results == null || results.isEmpty() ? null : results.getFirst(); } catch (SQLException e) { LOGGER.error( @@ -888,7 +939,7 @@ private List fetchPolicyMappingRecords(String query) ModelPolicyMappingRecord.class, ModelPolicyMappingRecord::toPolicyMappingRecord, null, - Integer.MAX_VALUE); + ReadEverythingPageToken.get()); return results == null ? Collections.emptyList() : results; } catch (SQLException e) { throw new RuntimeException( @@ -923,4 +974,10 @@ PolarisStorageIntegration loadPolarisStorageIntegration( BaseMetaStoreManager.extractStorageConfiguration(callContext, entity); return storageIntegrationProvider.getStorageIntegrationForConfig(storageConfig); } + + @Nonnull + @Override + public PageToken.PageTokenBuilder pageTokenBuilder() { + return EntityIdPageToken.builder(); + } } diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java index 0f981a99b2..f871f65eb6 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java @@ -100,7 +100,8 @@ private void initializeForRealm( databaseOperations, secretsGenerator(realmContext, rootCredentialsSet), storageIntegrationProvider, - realmContext.getRealmIdentifier())); + realmContext.getRealmIdentifier(), + diagServices)); PolarisMetaStoreManager metaStoreManager = createNewMetaStoreManager(); metaStoreManagerMap.put(realmContext.getRealmIdentifier(), metaStoreManager); diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java index 7d0b5ec928..896079780c 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/QueryGenerator.java @@ -26,13 +26,19 @@ import java.util.Map; import org.apache.polaris.core.entity.PolarisEntityCore; import org.apache.polaris.core.entity.PolarisEntityId; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelEntity; import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelGrantRecord; import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelPolicyMappingRecord; import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelPrincipalAuthenticationData; +import org.apache.polaris.jpa.models.EntityIdPageToken; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class QueryGenerator { + private static final Logger LOGGER = LoggerFactory.getLogger(QueryGenerator.class); public static String generateSelectQuery( @Nonnull Class entityClass, @Nonnull Map whereClause) { @@ -218,4 +224,17 @@ public static String getTableName(@Nonnull Class entityClass) { return tableName; } + + public static String updateQueryWithPageToken(String existingQuery, PageToken pageToken) { + if (pageToken instanceof ReadEverythingPageToken) { + return existingQuery; + } else if (pageToken instanceof EntityIdPageToken) { + long previousPageEntityId = ((EntityIdPageToken) pageToken).id; + return String.format("%s AND id > %d ORDER BY id ASC", existingQuery, previousPageEntityId); + } else { + // The caller of this method is supposed to already have validated the PageToken! + LOGGER.error("Unsupported page token: {}", pageToken); + return existingQuery; + } + } } diff --git a/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java index 92d31d8343..57df0febb9 100644 --- a/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java +++ b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java @@ -58,7 +58,12 @@ protected PolarisTestMetaStoreManager createPolarisTestMetaStoreManager() { } JdbcBasePersistenceImpl basePersistence = - new JdbcBasePersistenceImpl(datasourceOperations, RANDOM_SECRETS, Mockito.mock(), "REALM"); + new JdbcBasePersistenceImpl( + datasourceOperations, + RANDOM_SECRETS, + Mockito.mock(), + "REALM", + new PolarisDefaultDiagServiceImpl()); return new PolarisTestMetaStoreManager( new AtomicOperationMetaStoreManager(), new PolarisCallContext( diff --git a/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java index 48d2aa39bf..5544033330 100644 --- a/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java +++ b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java @@ -26,6 +26,7 @@ import java.sql.Statement; import java.util.function.Function; import javax.sql.DataSource; +import org.apache.polaris.core.persistence.pagination.ReadEverythingPageToken; import org.apache.polaris.extension.persistence.relational.jdbc.DatasourceOperations; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -78,7 +79,7 @@ void testExecuteSelect_exception() throws Exception { SQLException.class, () -> datasourceOperations.executeSelect( - query, Object.class, Function.identity(), null, Integer.MAX_VALUE)); + query, Object.class, Function.identity(), null, ReadEverythingPageToken.get())); } @Test diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/ReadEverythingPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/ReadEverythingPageToken.java index 5d6d296258..8a9edd3db9 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/ReadEverythingPageToken.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/ReadEverythingPageToken.java @@ -86,7 +86,7 @@ public PageToken updated(List newData) { /** {@link ReadEverythingPageToken} does not support page size */ @Override public PageToken withPageSize(Integer pageSize) { - if (pageSize == null) { + if (pageSize == null || pageSize == Integer.MAX_VALUE) { return ReadEverythingPageToken.get(); } else { throw new UnsupportedOperationException(); diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index 6f0acb87ff..86596df02a 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -2602,7 +2602,8 @@ public void testLookup() { this.polarisCallContext, null, PolarisEntityType.PRINCIPAL, - PolarisEntitySubType.NULL_SUBTYPE) + PolarisEntitySubType.NULL_SUBTYPE, + ReadEverythingPageToken.get()) .getEntities(); // ensure not null, one element only 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 67d374947b..5bb22cb78b 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 @@ -502,15 +502,11 @@ private boolean paginationEnabled() { @Override public List listTables(Namespace namespace) { -<<<<<<< HEAD return listTables(namespace, ReadEverythingPageToken.get()).data; } public PolarisPage listTables(Namespace namespace, PageToken pageToken) { if (!namespaceExists(namespace) && !namespace.isEmpty()) { -======= - if (!namespaceExists(namespace)) { ->>>>>>> cfe22b7de57bfeb6f877bee54b8a959f30173451 throw new NoSuchNamespaceException( "Cannot list tables for namespace. Namespace does not exist: '%s'", namespace); } @@ -873,15 +869,11 @@ public void close() throws IOException { @Override public List listViews(Namespace namespace) { -<<<<<<< HEAD return listViews(namespace, ReadEverythingPageToken.get()).data; } public PolarisPage listViews(Namespace namespace, PageToken pageToken) { if (!namespaceExists(namespace) && !namespace.isEmpty()) { -======= - if (!namespaceExists(namespace)) { ->>>>>>> cfe22b7de57bfeb6f877bee54b8a959f30173451 throw new NoSuchNamespaceException( "Cannot list views for namespace. Namespace does not exist: '%s'", namespace); } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index 4cccb3a6ac..cf9a21a724 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -90,12 +90,9 @@ import org.apache.polaris.core.persistence.TransactionWorkspaceMetaStoreManager; import org.apache.polaris.core.persistence.dao.entity.EntitiesResult; import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; -<<<<<<< HEAD import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.pagination.PolarisPage; -======= import org.apache.polaris.core.secrets.UserSecretsManager; ->>>>>>> cfe22b7de57bfeb6f877bee54b8a959f30173451 import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.SupportsNotifications; import org.apache.polaris.service.catalog.common.CatalogHandler; @@ -175,7 +172,6 @@ public static boolean isCreate(UpdateTableRequest request) { return isCreate; } -<<<<<<< HEAD public ListNamespacesResponseWithPageToken listNamespaces(Namespace parent, PageToken pageToken) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES; authorizeBasicNamespaceOperationOrThrow(op, parent); @@ -188,7 +184,8 @@ public ListNamespacesResponseWithPageToken listNamespaces(Namespace parent, Page PolarisPage.fromData( CatalogHandlers.listNamespaces(namespaceCatalog, parent).namespaces())); } -======= + } + private UserSecretsManager getUserSecretsManager() { return userSecretsManager; } @@ -238,7 +235,6 @@ protected void initializeCatalog() { this.namespaceCatalog = (baseCatalog instanceof SupportsNamespaces) ? (SupportsNamespaces) baseCatalog : null; this.viewCatalog = (baseCatalog instanceof ViewCatalog) ? (ViewCatalog) baseCatalog : null; ->>>>>>> cfe22b7de57bfeb6f877bee54b8a959f30173451 } public ListNamespacesResponse listNamespaces(Namespace parent) { From 6b9031c77321fd41d77166a2e86d1a7b78263677 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 28 Apr 2025 14:03:36 -0700 Subject: [PATCH 23/23] fix --- .../polaris/service/catalog/iceberg/IcebergCatalog.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 5bb22cb78b..a34d92cb08 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 @@ -506,7 +506,7 @@ public List listTables(Namespace namespace) { } public PolarisPage listTables(Namespace namespace, PageToken pageToken) { - if (!namespaceExists(namespace) && !namespace.isEmpty()) { + if (!namespaceExists(namespace)) { throw new NoSuchNamespaceException( "Cannot list tables for namespace. Namespace does not exist: '%s'", namespace); } @@ -873,7 +873,7 @@ public List listViews(Namespace namespace) { } public PolarisPage listViews(Namespace namespace, PageToken pageToken) { - if (!namespaceExists(namespace) && !namespace.isEmpty()) { + if (!namespaceExists(namespace)) { throw new NoSuchNamespaceException( "Cannot list views for namespace. Namespace does not exist: '%s'", namespace); }