From 3375ff5535735c8cce08e84165753bd11be1c441 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 9 May 2025 13:10:07 -0700 Subject: [PATCH 01/18] add pagetoken impl --- .../pagination/EntityIdPageToken.java | 85 +++++++++++++++++++ .../persistence/pagination/PageToken.java | 22 ++++- .../persistence/pagination/PageTokenTest.java | 34 ++++++++ 3 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java new file mode 100644 index 0000000000..04b526152e --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java @@ -0,0 +1,85 @@ +/* + * 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 org.apache.polaris.core.entity.PolarisBaseEntity; + +import java.util.List; + +public class EntityIdPageToken extends PageToken implements HasPageSize { + + public static final String PREFIX = "entity-id"; + + /** 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; + + private final long entityId; + private final int pageSize; + + public EntityIdPageToken(int pageSize) { + this.entityId = BASE_ID; + this.pageSize = pageSize; + } + + public EntityIdPageToken(long entityId, int pageSize) { + this.entityId = entityId; + this.pageSize = pageSize; + } + + public long getId() { + return entityId; + } + + @Override + public int getPageSize() { + return this.pageSize; + } + + @Override + public String toTokenString() { + return String.format("%s/%d/%d", PREFIX, entityId, pageSize); + } + + + /** + * Builds a new page token to reflect new data that's been read. + * This implementation assumes that the input list is sorted, and + * checks that it's a list of `PolarisBaseEntity` + */ + @Override + public PageToken updated(List newData) { + if (newData == null || newData.size() < this.pageSize) { + return new DonePageToken(); + } else { + var head = newData.get(0); + 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); + } else { + throw new IllegalArgumentException( + "Cannot build a page token from: " + newData.get(0).getClass().getSimpleName()); + } + } + } +} 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 index 2e335ccd40..bc40eb6ac1 100644 --- 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 @@ -18,8 +18,12 @@ */ package org.apache.polaris.core.persistence.pagination; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.util.List; import java.util.Objects; +import java.util.Optional; /** * Represents a page token that can be used by operations like `listTables`. Clients that specify a @@ -31,6 +35,7 @@ * response, that means there is no more data to read. */ public abstract class PageToken { + private static Logger LOGGER = LoggerFactory.getLogger(PageToken.class); /** Build a new PageToken that reads everything */ public static PageToken readEverything() { @@ -56,8 +61,21 @@ public static PageToken build(String token, Integer pageSize) { return new ReadEverythingPageToken(); } } else { - // TODO implement, split out by the token's prefix - throw new IllegalArgumentException("Unrecognized page token: " + token); + try { + String[] parts = token.split("/"); + if (parts.length < 1) { + throw new IllegalArgumentException("Invalid token format: " + token); + } else if (parts[0].equals(EntityIdPageToken.PREFIX)) { + int resolvedPageSize = pageSize == null ? Integer.parseInt(parts[2]) : pageSize; + return new EntityIdPageToken(Long.parseLong(parts[1]), resolvedPageSize); + } else { + throw new IllegalArgumentException("Unrecognized page token: " + token); + } + } catch (NumberFormatException | IndexOutOfBoundsException e) { + LOGGER.debug(e.getMessage()); + throw new IllegalArgumentException("Invalid token format: " + token); + } + } } 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 97e52fb842..db2b3aca09 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 @@ -18,7 +18,11 @@ */ package org.apache.polaris.service.persistence.pagination; +import org.apache.polaris.core.entity.PolarisBaseEntity; +import org.apache.polaris.core.entity.PolarisEntitySubType; +import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.persistence.pagination.DonePageToken; +import org.apache.polaris.core.persistence.pagination.EntityIdPageToken; import org.apache.polaris.core.persistence.pagination.HasPageSize; import org.apache.polaris.core.persistence.pagination.PageToken; import org.assertj.core.api.Assertions; @@ -26,6 +30,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.List; + public class PageTokenTest { private static final Logger LOGGER = LoggerFactory.getLogger(PageTokenTest.class); @@ -47,4 +53,32 @@ void testReadEverythingPageToken() { Assertions.assertThat(PageToken.readEverything()).isEqualTo(PageToken.readEverything()); } + + + @Test + void testEntityIdPageToken() { + EntityIdPageToken token = new EntityIdPageToken(2); + + Assertions.assertThat(token).isInstanceOf(EntityIdPageToken.class); + Assertions.assertThat(token.getId()).isEqualTo(-1L); + + List badData = List.of("some", "data"); + Assertions.assertThatThrownBy(() -> token.buildNextPage(badData)) + .isInstanceOf(IllegalArgumentException.class); + + List data = List.of( + new PolarisBaseEntity(0, 101, PolarisEntityType.NULL_TYPE, PolarisEntitySubType.ANY_SUBTYPE, 0, "101"), + new PolarisBaseEntity(0, 102, PolarisEntityType.NULL_TYPE, PolarisEntitySubType.ANY_SUBTYPE, 0, "102") + ); + var page = token.buildNextPage(data); + + Assertions.assertThat(page.pageToken).isNotNull(); + Assertions.assertThat(page.pageToken).isInstanceOf(EntityIdPageToken.class); + Assertions.assertThat(((EntityIdPageToken)page.pageToken).getPageSize()).isEqualTo(2); + Assertions.assertThat(((EntityIdPageToken) page.pageToken).getId()).isEqualTo(102); + Assertions.assertThat(page.items).isEqualTo(data); + + Assertions.assertThat(PageToken.fromString(page.pageToken.toTokenString())) + .isEqualTo(page.pageToken); + } } From 255e44b66ed86f05ec26708ef923038c2c6a76c5 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 9 May 2025 13:40:45 -0700 Subject: [PATCH 02/18] persistence impls --- .../eclipselink/PolarisEclipseLinkStore.java | 15 ++- .../jdbc/JdbcBasePersistenceImpl.java | 8 +- .../pagination/EntityIdPageToken.java | 96 +++++++++---------- .../pagination/LimitPageToken.java | 52 ---------- .../persistence/pagination/PageToken.java | 9 +- .../TreeMapTransactionalPersistenceImpl.java | 13 ++- .../persistence/pagination/PageTokenTest.java | 16 ++-- 7 files changed, 87 insertions(+), 122 deletions(-) delete mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/LimitPageToken.java 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 4e992e07f4..d39ebf6f97 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,8 @@ 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.EntityIdPageToken; +import org.apache.polaris.core.persistence.pagination.HasPageSize; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.jpa.models.ModelEntity; @@ -292,15 +294,22 @@ List lookupFullEntitiesActive( 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"; + String hql = "SELECT m from ModelEntity m where" + + " m.catalogId=:catalogId and m.parentId=:parentId and m.typeCode=:typeCode and m.id > :tokenId"; + + long tokenId = EntityIdPageToken.BASE_ID; + if (pageToken instanceof EntityIdPageToken entityIdPageToken) { + hql += " order by m.id asc"; + tokenId = entityIdPageToken.getId(); + } TypedQuery query = session .createQuery(hql, ModelEntity.class) .setParameter("catalogId", catalogId) .setParameter("parentId", parentId) - .setParameter("typeCode", entityType.getCode()); + .setParameter("typeCode", entityType.getCode()) + .setParameter("tokenId", tokenId); return query.getResultList(); } 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 cd6a0b6c07..0fe6fa959b 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 @@ -49,6 +49,7 @@ 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.EntityIdPageToken; import org.apache.polaris.core.persistence.pagination.HasPageSize; import org.apache.polaris.core.persistence.pagination.Page; import org.apache.polaris.core.persistence.pagination.PageToken; @@ -414,6 +415,11 @@ public Page listEntities( // Limit can't be pushed down, due to client side filtering // absence of transaction. String query = QueryGenerator.generateSelectQuery(new ModelEntity(), params); + + if (pageToken instanceof EntityIdPageToken entityIdPageToken) { + query += String.format("AND id > %d ORDER BY id ASC", entityIdPageToken.getId()); + } + try { List results = new ArrayList<>(); datasourceOperations.executeSelectOverStream( @@ -429,7 +435,7 @@ public Page listEntities( List resultsOrEmpty = results == null ? Collections.emptyList() - : results.stream().filter(entityFilter).map(transformer).collect(Collectors.toList()); + : results.stream().map(transformer).collect(Collectors.toList()); return Page.fromItems(resultsOrEmpty); } catch (SQLException e) { throw new RuntimeException( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java index 04b526152e..543c84e502 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java @@ -16,70 +16,66 @@ * specific language governing permissions and limitations * under the License. */ - package org.apache.polaris.core.persistence.pagination; -import org.apache.polaris.core.entity.PolarisBaseEntity; - import java.util.List; +import org.apache.polaris.core.entity.PolarisBaseEntity; public class EntityIdPageToken extends PageToken implements HasPageSize { - public static final String PREFIX = "entity-id"; - - /** The minimum ID that could be attached to an entity */ - private static final long MINIMUM_ID = 0; + public static final String PREFIX = "entity-id"; - /** The entity ID to use to start with. */ - private static final long BASE_ID = MINIMUM_ID - 1; + /** The minimum ID that could be attached to an entity */ + private static final long MINIMUM_ID = 0; - private final long entityId; - private final int pageSize; + /** The entity ID to use to start with. */ + public static final long BASE_ID = MINIMUM_ID - 1; - public EntityIdPageToken(int pageSize) { - this.entityId = BASE_ID; - this.pageSize = pageSize; - } + private final long entityId; + private final int pageSize; - public EntityIdPageToken(long entityId, int pageSize) { - this.entityId = entityId; - this.pageSize = pageSize; - } + public EntityIdPageToken(int pageSize) { + this.entityId = BASE_ID; + this.pageSize = pageSize; + } - public long getId() { - return entityId; - } + public EntityIdPageToken(long entityId, int pageSize) { + this.entityId = entityId; + this.pageSize = pageSize; + } - @Override - public int getPageSize() { - return this.pageSize; - } + public long getId() { + return entityId; + } - @Override - public String toTokenString() { - return String.format("%s/%d/%d", PREFIX, entityId, pageSize); - } + @Override + public int getPageSize() { + return this.pageSize; + } + @Override + public String toTokenString() { + return String.format("%s/%d/%d", PREFIX, entityId, pageSize); + } - /** - * Builds a new page token to reflect new data that's been read. - * This implementation assumes that the input list is sorted, and - * checks that it's a list of `PolarisBaseEntity` - */ - @Override - public PageToken updated(List newData) { - if (newData == null || newData.size() < this.pageSize) { - return new DonePageToken(); - } else { - var head = newData.get(0); - 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); - } else { - throw new IllegalArgumentException( - "Cannot build a page token from: " + newData.get(0).getClass().getSimpleName()); - } - } + /** + * Builds a new page token to reflect new data that's been read. This implementation assumes that + * the input list is sorted, and checks that it's a list of `PolarisBaseEntity` + */ + @Override + public PageToken updated(List newData) { + if (newData == null || newData.size() < this.pageSize) { + return new DonePageToken(); + } else { + var head = newData.get(0); + 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); + } else { + throw new IllegalArgumentException( + "Cannot build a page token from: " + newData.get(0).getClass().getSimpleName()); + } } + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/LimitPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/LimitPageToken.java deleted file mode 100644 index 18586446ca..0000000000 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/LimitPageToken.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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 that has a page size, but no start offset. This can be used to - * represent a `limit`. When updated, it returns {@link DonePageToken}. As such it should never be - * user-facing and doesn't truly paginate. - */ -public class LimitPageToken extends PageToken implements HasPageSize { - - public static final String PREFIX = "limit"; - - private final int pageSize; - - public LimitPageToken(int pageSize) { - this.pageSize = pageSize; - } - - @Override - public int getPageSize() { - return pageSize; - } - - @Override - public String toTokenString() { - return String.format("%s/%d", PREFIX, pageSize); - } - - @Override - protected PageToken updated(List newData) { - return new DonePageToken(); - } -} 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 index bc40eb6ac1..90bb811f90 100644 --- 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 @@ -18,12 +18,10 @@ */ package org.apache.polaris.core.persistence.pagination; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.util.List; import java.util.Objects; -import java.util.Optional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Represents a page token that can be used by operations like `listTables`. Clients that specify a @@ -56,7 +54,7 @@ public static PageToken fromLimit(Integer pageSize) { public static PageToken build(String token, Integer pageSize) { if (token == null || token.isEmpty()) { if (pageSize != null) { - return new LimitPageToken(pageSize); + return new EntityIdPageToken(pageSize); } else { return new ReadEverythingPageToken(); } @@ -75,7 +73,6 @@ public static PageToken build(String token, Integer pageSize) { LOGGER.debug(e.getMessage()); throw new IllegalArgumentException("Invalid token format: " + token); } - } } 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 304ac0ce97..3929027ace 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,6 +21,8 @@ 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; @@ -39,6 +41,7 @@ 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.EntityIdPageToken; import org.apache.polaris.core.persistence.pagination.HasPageSize; import org.apache.polaris.core.persistence.pagination.Page; import org.apache.polaris.core.persistence.pagination.PageToken; @@ -360,8 +363,14 @@ public List lookupEntityActiveBatchInCurrentTxn( .map( nameRecord -> this.lookupEntityInCurrentTxn( - callCtx, catalogId, nameRecord.getId(), entityType.getCode())) - .filter(entityFilter); + callCtx, catalogId, nameRecord.getId(), entityType.getCode())); + + if (pageToken instanceof EntityIdPageToken) { + data = data.sorted(Comparator.comparingLong(PolarisEntityCore::getId)); + } + + data = data.filter(entityFilter); + if (pageToken instanceof HasPageSize) { data = data.limit(((HasPageSize) pageToken).getPageSize()); } 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 db2b3aca09..b2a709b8ea 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 @@ -18,6 +18,7 @@ */ package org.apache.polaris.service.persistence.pagination; +import java.util.List; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; @@ -30,8 +31,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.List; - public class PageTokenTest { private static final Logger LOGGER = LoggerFactory.getLogger(PageTokenTest.class); @@ -54,7 +53,6 @@ void testReadEverythingPageToken() { Assertions.assertThat(PageToken.readEverything()).isEqualTo(PageToken.readEverything()); } - @Test void testEntityIdPageToken() { EntityIdPageToken token = new EntityIdPageToken(2); @@ -66,15 +64,17 @@ void testEntityIdPageToken() { Assertions.assertThatThrownBy(() -> token.buildNextPage(badData)) .isInstanceOf(IllegalArgumentException.class); - List data = List.of( - new PolarisBaseEntity(0, 101, PolarisEntityType.NULL_TYPE, PolarisEntitySubType.ANY_SUBTYPE, 0, "101"), - new PolarisBaseEntity(0, 102, PolarisEntityType.NULL_TYPE, PolarisEntitySubType.ANY_SUBTYPE, 0, "102") - ); + List data = + List.of( + new PolarisBaseEntity( + 0, 101, PolarisEntityType.NULL_TYPE, PolarisEntitySubType.ANY_SUBTYPE, 0, "101"), + new PolarisBaseEntity( + 0, 102, PolarisEntityType.NULL_TYPE, PolarisEntitySubType.ANY_SUBTYPE, 0, "102")); var page = token.buildNextPage(data); Assertions.assertThat(page.pageToken).isNotNull(); Assertions.assertThat(page.pageToken).isInstanceOf(EntityIdPageToken.class); - Assertions.assertThat(((EntityIdPageToken)page.pageToken).getPageSize()).isEqualTo(2); + Assertions.assertThat(((EntityIdPageToken) page.pageToken).getPageSize()).isEqualTo(2); Assertions.assertThat(((EntityIdPageToken) page.pageToken).getId()).isEqualTo(102); Assertions.assertThat(page.items).isEqualTo(data); From 015e637d138d0abc480664364d96f2220a5ac725 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 9 May 2025 14:00:41 -0700 Subject: [PATCH 03/18] stable --- .../eclipselink/PolarisEclipseLinkStore.java | 6 +- .../pagination/EntityIdPageToken.java | 17 ++- .../TreeMapTransactionalPersistenceImpl.java | 5 +- .../quarkus/catalog/IcebergCatalogTest.java | 138 ++++++++++++++++++ .../catalog/iceberg/IcebergCatalog.java | 1 - 5 files changed, 154 insertions(+), 13 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 d39ebf6f97..e4e1579759 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 @@ -36,7 +36,6 @@ import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.pagination.EntityIdPageToken; -import org.apache.polaris.core.persistence.pagination.HasPageSize; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.jpa.models.ModelEntity; @@ -294,8 +293,9 @@ List lookupFullEntitiesActive( 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 and m.id > :tokenId"; + String hql = + "SELECT m from ModelEntity m where" + + " m.catalogId=:catalogId and m.parentId=:parentId and m.typeCode=:typeCode and m.id > :tokenId"; long tokenId = EntityIdPageToken.BASE_ID; if (pageToken instanceof EntityIdPageToken entityIdPageToken) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java index 543c84e502..bd7decec0d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java @@ -19,6 +19,7 @@ package org.apache.polaris.core.persistence.pagination; import java.util.List; +import org.apache.polaris.core.entity.EntityNameLookupRecord; import org.apache.polaris.core.entity.PolarisBaseEntity; public class EntityIdPageToken extends PageToken implements HasPageSize { @@ -60,21 +61,23 @@ public String toTokenString() { /** * Builds a new page token to reflect new data that's been read. This implementation assumes that - * the input list is sorted, and checks that it's a list of `PolarisBaseEntity` + * the input list is sorted, and checks that it's a list of `PolarisBaseEntity` or + * `EntityNameLookupRecord` */ @Override public PageToken updated(List newData) { if (newData == null || newData.size() < this.pageSize) { return new DonePageToken(); } else { - var head = newData.get(0); - 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); + // Assumed to be sorted with the greatest entity ID last + var tail = newData.get(newData.size() - 1); + if (tail instanceof PolarisBaseEntity) { + return new EntityIdPageToken(((PolarisBaseEntity) tail).getId(), this.pageSize); + } else if (tail instanceof EntityNameLookupRecord) { + return new EntityIdPageToken(((EntityNameLookupRecord) tail).getId(), this.pageSize); } else { throw new IllegalArgumentException( - "Cannot build a page token from: " + newData.get(0).getClass().getSimpleName()); + "Cannot build a page token from: " + tail.getClass().getSimpleName()); } } } 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 3929027ace..b5bc739d1b 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,7 +21,6 @@ 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; @@ -366,7 +365,9 @@ public List lookupEntityActiveBatchInCurrentTxn( callCtx, catalogId, nameRecord.getId(), entityType.getCode())); if (pageToken instanceof EntityIdPageToken) { - data = data.sorted(Comparator.comparingLong(PolarisEntityCore::getId)); + data = + data.sorted(Comparator.comparingLong(PolarisEntityCore::getId)) + .filter(e -> e.getId() > ((EntityIdPageToken) pageToken).getId()); } data = data.filter(entityFilter); 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 5837f88223..df8e75cf58 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 @@ -97,6 +97,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.Page; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.transactional.TransactionalPersistence; import org.apache.polaris.core.secrets.UserSecretsManager; @@ -1975,6 +1976,143 @@ public void testEventsAreEmitted() { Assertions.assertThat(afterTableEvent.metadata().properties().get(key)).isEqualTo(valNew); } + @Test + public void testPaginatedListTables() { + Assumptions.assumeTrue( + requiresNamespaceCreate(), + "Only applicable if namespaces must be created before adding children"); + + 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: + Page firstListResult = catalog.listTables(NS, null, 2); + Assertions.assertThat(firstListResult.items.size()).isEqualTo(2); + Assertions.assertThat(firstListResult.pageToken.toTokenString()).isNotNull().isNotEmpty(); + + // List using the previously obtained token: + Page secondListResult = + catalog.listTables(NS, firstListResult.pageToken.toTokenString(), null); + Assertions.assertThat(secondListResult.items.size()).isEqualTo(2); + Assertions.assertThat(secondListResult.pageToken.toTokenString()).isNotNull().isNotEmpty(); + + // List using the final token: + Page finalListResult = + catalog.listTables(NS, secondListResult.pageToken.toTokenString(), null); + Assertions.assertThat(finalListResult.items.size()).isEqualTo(1); + Assertions.assertThat(finalListResult.pageToken.toTokenString()).isNull(); + } finally { + for (int i = 0; i < 5; i++) { + catalog.dropTable(TableIdentifier.of(NS, "pagination_table_" + i)); + } + } + } + + @Test + public void testPaginatedListViews() { + Assumptions.assumeTrue( + requiresNamespaceCreate(), + "Only applicable if namespaces must be created before adding children"); + + 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: + Page firstListResult = catalog.listViews(NS, null, 2); + Assertions.assertThat(firstListResult.items.size()).isEqualTo(2); + Assertions.assertThat(firstListResult.pageToken.toTokenString()).isNotNull().isNotEmpty(); + + // List using the previously obtained token: + Page secondListResult = + catalog.listViews(NS, firstListResult.pageToken.toTokenString(), null); + Assertions.assertThat(secondListResult.items.size()).isEqualTo(2); + Assertions.assertThat(secondListResult.pageToken.toTokenString()).isNotNull().isNotEmpty(); + + // List using the final token: + Page finalListResult = + catalog.listViews(NS, secondListResult.pageToken.toTokenString(), null); + Assertions.assertThat(finalListResult.items.size()).isEqualTo(1); + Assertions.assertThat(finalListResult.pageToken.toTokenString()).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: + Page firstListResult = catalog.listNamespaces(Namespace.empty(), null, 2); + Assertions.assertThat(firstListResult.items.size()).isEqualTo(2); + Assertions.assertThat(firstListResult.pageToken.toTokenString()).isNotNull().isNotEmpty(); + + // List using the previously obtained token: + Page secondListResult = + catalog.listNamespaces( + Namespace.empty(), firstListResult.pageToken.toTokenString(), null); + Assertions.assertThat(secondListResult.items.size()).isEqualTo(2); + Assertions.assertThat(secondListResult.pageToken.toTokenString()).isNotNull().isNotEmpty(); + + // List using the final token: + Page finalListResult = + catalog.listNamespaces( + Namespace.empty(), secondListResult.pageToken.toTokenString(), null); + Assertions.assertThat(finalListResult.items.size()).isEqualTo(1); + Assertions.assertThat(finalListResult.pageToken.toTokenString()).isNull(); + + // List with page size matching the amount of data + Page firstExactListResult = catalog.listNamespaces(Namespace.empty(), null, 5); + Assertions.assertThat(firstExactListResult.items.size()).isEqualTo(5); + Assertions.assertThat(firstExactListResult.pageToken.toTokenString()) + .isNotNull() + .isNotEmpty(); + + // Again list with matching page size + Page secondExactListResult = + catalog.listNamespaces( + Namespace.empty(), firstExactListResult.pageToken.toTokenString(), null); + Assertions.assertThat(secondExactListResult.items).isEmpty(); + Assertions.assertThat(secondExactListResult.pageToken.toTokenString()).isNull(); + + // List with huge page size: + Page bigListResult = catalog.listNamespaces(Namespace.empty(), null, 9999); + Assertions.assertThat(bigListResult.items.size()).isEqualTo(5); + Assertions.assertThat(bigListResult.pageToken.toTokenString()).isNull(); + } finally { + for (int i = 0; i < 5; i++) { + catalog.dropNamespace(Namespace.of("pagination_namespace_" + i)); + } + } + } + private static InMemoryFileIO getInMemoryIo(IcebergCatalog catalog) { return (InMemoryFileIO) ((ExceptionMappingFileIO) catalog.getIo()).getInnerIo(); } 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 7c02a61549..df6b521a85 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 @@ -2569,7 +2569,6 @@ private int getMaxMetadataRefreshRetries() { /** Build a {@link PageToken} from a string and page size. */ private PageToken buildPageToken(@Nullable String tokenString, @Nullable Integer pageSize) { - boolean paginationEnabled = callContext .getPolarisCallContext() From 434ffb15d51e18fb7307e5a09c20c19244ef32c7 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 9 May 2025 14:12:50 -0700 Subject: [PATCH 04/18] another test --- .../persistence/pagination/PageTokenTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 b2a709b8ea..d8a68ad7a0 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 @@ -81,4 +81,18 @@ void testEntityIdPageToken() { Assertions.assertThat(PageToken.fromString(page.pageToken.toTokenString())) .isEqualTo(page.pageToken); } + + @Test + void testInvalidPageTokens() { + Assertions + .assertThatCode(() -> PageToken.fromString("not-real")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Unrecognized page token"); + + PageToken goodToken = PageToken.fromLimit(100); + Assertions + .assertThatCode(() -> PageToken.fromString(goodToken.toTokenString() + "???")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid token format"); + } } From 8ffc29dc8907dccf37501535da291880114f4bf7 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 9 May 2025 14:18:10 -0700 Subject: [PATCH 05/18] another small test --- .../service/persistence/pagination/PageTokenTest.java | 5 +++++ 1 file changed, 5 insertions(+) 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 d8a68ad7a0..33052a295f 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 @@ -94,5 +94,10 @@ void testInvalidPageTokens() { .assertThatCode(() -> PageToken.fromString(goodToken.toTokenString() + "???")) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Invalid token format"); + + Assertions + .assertThatCode(() -> PageToken.fromString(EntityIdPageToken.PREFIX + "/1")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid token format"); } } From 66d20aa03d95c9f1ebe30f3f8151167bc019cd3b Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 9 May 2025 14:18:13 -0700 Subject: [PATCH 06/18] autolint --- .../service/persistence/pagination/PageTokenTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) 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 33052a295f..2449aa70f1 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 @@ -84,19 +84,16 @@ void testEntityIdPageToken() { @Test void testInvalidPageTokens() { - Assertions - .assertThatCode(() -> PageToken.fromString("not-real")) + Assertions.assertThatCode(() -> PageToken.fromString("not-real")) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Unrecognized page token"); PageToken goodToken = PageToken.fromLimit(100); - Assertions - .assertThatCode(() -> PageToken.fromString(goodToken.toTokenString() + "???")) + Assertions.assertThatCode(() -> PageToken.fromString(goodToken.toTokenString() + "???")) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Invalid token format"); - Assertions - .assertThatCode(() -> PageToken.fromString(EntityIdPageToken.PREFIX + "/1")) + Assertions.assertThatCode(() -> PageToken.fromString(EntityIdPageToken.PREFIX + "/1")) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Invalid token format"); } From a5d62ed83f6deac26d93780472604fc2f43c3b7c Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 9 May 2025 19:47:05 -0700 Subject: [PATCH 07/18] typofix --- .../persistence/relational/jdbc/JdbcBasePersistenceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0fe6fa959b..7691f18d1a 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 @@ -417,7 +417,7 @@ public Page listEntities( String query = QueryGenerator.generateSelectQuery(new ModelEntity(), params); if (pageToken instanceof EntityIdPageToken entityIdPageToken) { - query += String.format("AND id > %d ORDER BY id ASC", entityIdPageToken.getId()); + query += String.format(" AND id > %d ORDER BY id ASC", entityIdPageToken.getId()); } try { From cfdf11eaff2b1c4dc5d114c8555e5e530b840d57 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 16:25:18 -0700 Subject: [PATCH 08/18] changes per review --- .../relational/jdbc/JdbcBasePersistenceImpl.java | 11 ++++++++++- .../relational/jdbc/JdbcMetaStoreManagerFactory.java | 3 ++- ...tastoreManagerWithJdbcBasePersistenceImplTest.java | 3 ++- .../persistence/pagination/EntityIdPageToken.java | 2 +- .../service/persistence/pagination/PageTokenTest.java | 3 ++- 5 files changed, 17 insertions(+), 5 deletions(-) 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 3af4f0c214..8a7e2cf5a9 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 @@ -34,6 +34,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; @@ -53,6 +54,7 @@ import org.apache.polaris.core.persistence.pagination.HasPageSize; import org.apache.polaris.core.persistence.pagination.Page; import org.apache.polaris.core.persistence.pagination.PageToken; +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; @@ -73,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 @@ -400,6 +405,10 @@ public Page listEntities( @Nonnull Predicate entityFilter, @Nonnull Function transformer, @Nonnull PageToken pageToken) { + polarisDiagnostics.check( + (pageToken instanceof EntityIdPageToken || pageToken instanceof ReadEverythingPageToken), + "unrecognized_page_token"); + Map params = Map.of( "catalog_id", 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 2fc1d4c5f1..400d52e08f 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 @@ -102,7 +102,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/test/java/org/apache/polaris/extension/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java index 1012aff02d..fe51d19a1d 100644 --- a/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java +++ b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java @@ -57,7 +57,8 @@ protected PolarisTestMetaStoreManager createPolarisTestMetaStoreManager() { } JdbcBasePersistenceImpl basePersistence = - new JdbcBasePersistenceImpl(datasourceOperations, RANDOM_SECRETS, Mockito.mock(), "REALM"); + new JdbcBasePersistenceImpl( + datasourceOperations, RANDOM_SECRETS, Mockito.mock(), "REALM", diagServices); return new PolarisTestMetaStoreManager( new AtomicOperationMetaStoreManager(), new PolarisCallContext( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java index bd7decec0d..11a7d96b56 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java @@ -76,7 +76,7 @@ public PageToken updated(List newData) { } else if (tail instanceof EntityNameLookupRecord) { return new EntityIdPageToken(((EntityNameLookupRecord) tail).getId(), this.pageSize); } else { - throw new IllegalArgumentException( + throw new IllegalStateException( "Cannot build a page token from: " + tail.getClass().getSimpleName()); } } 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 2449aa70f1..a21eb07cfe 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 @@ -60,9 +60,10 @@ void testEntityIdPageToken() { Assertions.assertThat(token).isInstanceOf(EntityIdPageToken.class); Assertions.assertThat(token.getId()).isEqualTo(-1L); + // EntityIdPageToken can only build a new page from certain types that have an Entity ID List badData = List.of("some", "data"); Assertions.assertThatThrownBy(() -> token.buildNextPage(badData)) - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalStateException.class); List data = List.of( From 903b94746334ede16b3e41391ebbf1948cb0e8eb Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 19:16:36 -0700 Subject: [PATCH 09/18] add new integration test --- .../polaris/service/it/env/CatalogApi.java | 17 ++++++++++ .../PolarisRestCatalogIntegrationTest.java | 34 +++++++++++++++++++ .../core/config/FeatureConfiguration.java | 1 + .../quarkus/catalog/IcebergCatalogTest.java | 4 ++- .../persistence/pagination/PageTokenTest.java | 4 +++ 5 files changed, 59 insertions(+), 1 deletion(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java index 7be67f1947..c0459c712a 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java @@ -101,6 +101,23 @@ public List listNamespaces(String catalog, Namespace parent) { } } + public ListNamespacesResponse listNamespaces(String catalog, Namespace parent, String pageToken, String pageSize) { + Map queryParams = new HashMap<>(); + if (!parent.isEmpty()) { + // TODO change this for Iceberg 1.7.2: + // queryParams.put("parent", RESTUtil.encodeNamespace(parent)); + queryParams.put("parent", Joiner.on('\u001f').join(parent.levels())); + } + queryParams.put("page-token", pageToken); + queryParams.put("page-size", pageSize); + try (Response response = + request("v1/{cat}/namespaces", Map.of("cat", catalog), queryParams).get()) { + assertThat(response.getStatus()).isEqualTo(OK.getStatusCode()); + ListNamespacesResponse res = response.readEntity(ListNamespacesResponse.class); + return res; + } + } + public List listAllNamespacesChildFirst(String catalog) { List result = new ArrayList<>(); for (int idx = -1; idx < result.size(); idx++) { diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index ae42c5ae5c..94938dd109 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -67,6 +67,7 @@ import org.apache.iceberg.rest.RESTUtil; import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.responses.ErrorResponse; +import org.apache.iceberg.rest.responses.ListNamespacesResponse; import org.apache.iceberg.types.Types; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.Catalog; @@ -1556,4 +1557,37 @@ public void testUpdateTableWithReservedProperty() { .hasMessageContaining("reserved prefix"); genericTableApi.purge(currentCatalogName, namespace); } + + @Test + public void testPaginatedListNamespaces() { + String prefix = "testPaginatedListNamespaces"; + for (int i = 0; i < 100; i++) { + Namespace namespace = Namespace.of(prefix + i); + restCatalog.createNamespace(namespace); + } + + try { + Assertions.assertThat(catalogApi.listNamespaces(currentCatalogName, Namespace.empty())).hasSize(100); + for (var pageSize: List.of(1, 2, 3, 49, 50, 51, 99, 100, 101, 1000)) { + int total = 0; + String pageToken = null; + do { + ListNamespacesResponse response = catalogApi + .listNamespaces(currentCatalogName, Namespace.empty(), pageToken, String.valueOf(pageSize)); + Assertions.assertThat(response.namespaces().size()).isLessThanOrEqualTo(pageSize); + total += response.namespaces().size(); + pageToken = response.nextPageToken(); + } while (pageToken != null); + Assertions + .assertThat(total) + .as("Total paginated results for pageSize = " + pageSize) + .isEqualTo(100); + } + } finally { + for (int i = 0; i < 100; i++) { + Namespace namespace = Namespace.of(prefix + i); + restCatalog.dropNamespace(namespace); + } + } + } } 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 39a342e95a..29286abb71 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 @@ -218,6 +218,7 @@ public static void enforceFeatureEnabledOrThrow( public static final PolarisConfiguration LIST_PAGINATION_ENABLED = PolarisConfiguration.builder() .key("LIST_PAGINATION_ENABLED") + .catalogConfig("polaris.config.list-pagination-enabled") .description("If set to true, pagination for APIs like listTables is enabled.") .defaultValue(false) .buildFeatureConfiguration(); 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 836b18fc6e..847c82ec9f 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 @@ -179,7 +179,9 @@ public Map getConfigOverrides() { "polaris.features.defaults.\"LIST_PAGINATION_ENABLED\"", "true", "polaris.event-listener.type", - "test"); + "test", + "LIST_PAGINATION_ENABLED", + "true"); } } 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 a21eb07cfe..27f876320c 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 @@ -51,6 +51,10 @@ void testReadEverythingPageToken() { Assertions.assertThat(token).isNotInstanceOf(HasPageSize.class); Assertions.assertThat(PageToken.readEverything()).isEqualTo(PageToken.readEverything()); + + Assertions + .assertThat(PageToken.readEverything().buildNextPage(List.of()).pageToken) + .isInstanceOf(DonePageToken.class); } @Test From f566fd1458560c4646508d1ced4ab95b38705356 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 19:18:57 -0700 Subject: [PATCH 10/18] hold up --- .../service/it/test/PolarisRestCatalogIntegrationTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index 94938dd109..ac8ab7d0ea 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -165,7 +165,8 @@ public class PolarisRestCatalogIntegrationTest extends CatalogTests private static final String[] DEFAULT_CATALOG_PROPERTIES = { "allow.unstructured.table.location", "true", - "allow.external.table.location", "true" + "allow.external.table.location", "true", + "polaris.config.list-pagination-enabled", "true" }; @Retention(RetentionPolicy.RUNTIME) From fea135d9f7b0d2e1dcb532f4872703fed4a96213 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 19:20:07 -0700 Subject: [PATCH 11/18] improve null check --- .../polaris/core/config/PolarisConfigurationStore.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java index acb171ce30..662a88d72d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java @@ -118,9 +118,14 @@ public interface PolarisConfigurationStore { PolarisConfiguration config) { if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) { Map propertiesMap = catalogEntity.getPropertiesAsMap(); - String propertyValue = propertiesMap.get(config.catalogConfig()); + String propertyValue = null; + if (config.hasCatalogConfig()) { + propertyValue = propertiesMap.get(config.catalogConfig()); + } if (propertyValue == null) { - propertyValue = propertiesMap.get(config.catalogConfigUnsafe()); + if (config.hasCatalogConfigUnsafe()) { + propertyValue = propertiesMap.get(config.catalogConfigUnsafe()); + } if (propertyValue != null) { LOGGER.warn( String.format( From 99e2a3768eb815a10cc4d3a9081d5c27f7e349ed Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 19:31:02 -0700 Subject: [PATCH 12/18] add a test --- .../config/DefaultConfigurationStoreTest.java | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index e533d52142..cd72a53a24 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -30,13 +30,16 @@ import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; +import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; import org.apache.polaris.service.config.FeaturesConfiguration; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; @@ -230,4 +233,51 @@ public void testInjectedFeaturesConfiguration() { assertThat(featuresConfiguration.realmOverrides().get(realmOne).overrides()) .containsKey(falseByDefaultKey); } + + @Test + public void testRegisterAndUseFeatureConfigurations() { + String prefix = "testRegisterAndUseFeatureConfigurations"; + + FeatureConfiguration safeConfig = FeatureConfiguration.builder() + .key(String.format("%s_safe", prefix)) + .catalogConfig(String.format("polaris.config.%s.safe", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); + + FeatureConfiguration unsafeConfig =FeatureConfiguration.builder() + .key(String.format("%s_unsafe", prefix)) + .catalogConfigUnsafe(String.format("%s.unsafe", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); + + FeatureConfiguration bothConfig = FeatureConfiguration.builder() + .key(String.format("%s_both", prefix)) + .catalogConfig(String.format("polaris.config.%s.both", prefix)) + .catalogConfigUnsafe(String.format("%s.both", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); + + CatalogEntity catalog = new CatalogEntity.Builder().build(); + + Assertions.assertThat(configurationStore + .getConfiguration( + polarisContext, + catalog, + safeConfig)).isTrue(); + + Assertions.assertThat(configurationStore + .getConfiguration( + polarisContext, + catalog, + unsafeConfig)).isTrue(); + + Assertions.assertThat(configurationStore + .getConfiguration( + polarisContext, + catalog, + bothConfig)).isTrue(); + } } From 29082076cd38a988584b6195d1547b99f7f807ed Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 19:31:05 -0700 Subject: [PATCH 13/18] autolint --- .../config/DefaultConfigurationStoreTest.java | 71 +++++++++---------- 1 file changed, 33 insertions(+), 38 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index cd72a53a24..ddd1026d97 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -238,46 +238,41 @@ public void testInjectedFeaturesConfiguration() { public void testRegisterAndUseFeatureConfigurations() { String prefix = "testRegisterAndUseFeatureConfigurations"; - FeatureConfiguration safeConfig = FeatureConfiguration.builder() - .key(String.format("%s_safe", prefix)) - .catalogConfig(String.format("polaris.config.%s.safe", prefix)) - .defaultValue(true) - .description(prefix) - .buildFeatureConfiguration(); - - FeatureConfiguration unsafeConfig =FeatureConfiguration.builder() - .key(String.format("%s_unsafe", prefix)) - .catalogConfigUnsafe(String.format("%s.unsafe", prefix)) - .defaultValue(true) - .description(prefix) - .buildFeatureConfiguration(); - - FeatureConfiguration bothConfig = FeatureConfiguration.builder() - .key(String.format("%s_both", prefix)) - .catalogConfig(String.format("polaris.config.%s.both", prefix)) - .catalogConfigUnsafe(String.format("%s.both", prefix)) - .defaultValue(true) - .description(prefix) - .buildFeatureConfiguration(); + FeatureConfiguration safeConfig = + FeatureConfiguration.builder() + .key(String.format("%s_safe", prefix)) + .catalogConfig(String.format("polaris.config.%s.safe", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); + + FeatureConfiguration unsafeConfig = + FeatureConfiguration.builder() + .key(String.format("%s_unsafe", prefix)) + .catalogConfigUnsafe(String.format("%s.unsafe", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); + + FeatureConfiguration bothConfig = + FeatureConfiguration.builder() + .key(String.format("%s_both", prefix)) + .catalogConfig(String.format("polaris.config.%s.both", prefix)) + .catalogConfigUnsafe(String.format("%s.both", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); CatalogEntity catalog = new CatalogEntity.Builder().build(); - Assertions.assertThat(configurationStore - .getConfiguration( - polarisContext, - catalog, - safeConfig)).isTrue(); - - Assertions.assertThat(configurationStore - .getConfiguration( - polarisContext, - catalog, - unsafeConfig)).isTrue(); - - Assertions.assertThat(configurationStore - .getConfiguration( - polarisContext, - catalog, - bothConfig)).isTrue(); + Assertions.assertThat(configurationStore.getConfiguration(polarisContext, catalog, safeConfig)) + .isTrue(); + + Assertions.assertThat( + configurationStore.getConfiguration(polarisContext, catalog, unsafeConfig)) + .isTrue(); + + Assertions.assertThat(configurationStore.getConfiguration(polarisContext, catalog, bothConfig)) + .isTrue(); } } From 6d2047e8ed1692a46b122b93943fc2b3d165a973 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 20:19:45 -0700 Subject: [PATCH 14/18] add integration test --- .../polaris/service/it/env/CatalogApi.java | 23 ++++++-- .../PolarisRestCatalogIntegrationTest.java | 54 +++++++++++++++---- .../persistence/pagination/PageTokenTest.java | 3 +- 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java index c0459c712a..5e10081d8c 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java @@ -101,17 +101,18 @@ public List listNamespaces(String catalog, Namespace parent) { } } - public ListNamespacesResponse listNamespaces(String catalog, Namespace parent, String pageToken, String pageSize) { + public ListNamespacesResponse listNamespaces( + String catalog, Namespace parent, String pageToken, String pageSize) { Map queryParams = new HashMap<>(); if (!parent.isEmpty()) { // TODO change this for Iceberg 1.7.2: // queryParams.put("parent", RESTUtil.encodeNamespace(parent)); queryParams.put("parent", Joiner.on('\u001f').join(parent.levels())); } - queryParams.put("page-token", pageToken); - queryParams.put("page-size", pageSize); + queryParams.put("pageToken", pageToken); + queryParams.put("pageSize", pageSize); try (Response response = - request("v1/{cat}/namespaces", Map.of("cat", catalog), queryParams).get()) { + request("v1/{cat}/namespaces", Map.of("cat", catalog), queryParams).get()) { assertThat(response.getStatus()).isEqualTo(OK.getStatusCode()); ListNamespacesResponse res = response.readEntity(ListNamespacesResponse.class); return res; @@ -159,6 +160,20 @@ public List listTables(String catalog, Namespace namespace) { } } + public ListTablesResponse listTables( + String catalog, Namespace namespace, String pageToken, String pageSize) { + String ns = RESTUtil.encodeNamespace(namespace); + Map queryParams = new HashMap<>(); + queryParams.put("pageToken", pageToken); + queryParams.put("pageSize", pageSize); + try (Response res = + request("v1/{cat}/namespaces/" + ns + "/tables", Map.of("cat", catalog), queryParams) + .get()) { + assertThat(res.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + return res.readEntity(ListTablesResponse.class); + } + } + public void dropTable(String catalog, TableIdentifier id) { String ns = RESTUtil.encodeNamespace(id.namespace()); try (Response res = diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java index ac8ab7d0ea..abb21c73bc 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java @@ -68,6 +68,7 @@ import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.responses.ErrorResponse; import org.apache.iceberg.rest.responses.ListNamespacesResponse; +import org.apache.iceberg.rest.responses.ListTablesResponse; import org.apache.iceberg.types.Types; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.Catalog; @@ -1562,33 +1563,68 @@ public void testUpdateTableWithReservedProperty() { @Test public void testPaginatedListNamespaces() { String prefix = "testPaginatedListNamespaces"; - for (int i = 0; i < 100; i++) { + for (int i = 0; i < 20; i++) { Namespace namespace = Namespace.of(prefix + i); restCatalog.createNamespace(namespace); } try { - Assertions.assertThat(catalogApi.listNamespaces(currentCatalogName, Namespace.empty())).hasSize(100); - for (var pageSize: List.of(1, 2, 3, 49, 50, 51, 99, 100, 101, 1000)) { + Assertions.assertThat(catalogApi.listNamespaces(currentCatalogName, Namespace.empty())) + .hasSize(20); + for (var pageSize : List.of(1, 2, 3, 9, 10, 11, 19, 20, 21, 2000)) { int total = 0; String pageToken = null; do { - ListNamespacesResponse response = catalogApi - .listNamespaces(currentCatalogName, Namespace.empty(), pageToken, String.valueOf(pageSize)); + ListNamespacesResponse response = + catalogApi.listNamespaces( + currentCatalogName, Namespace.empty(), pageToken, String.valueOf(pageSize)); Assertions.assertThat(response.namespaces().size()).isLessThanOrEqualTo(pageSize); total += response.namespaces().size(); pageToken = response.nextPageToken(); } while (pageToken != null); - Assertions - .assertThat(total) + Assertions.assertThat(total) .as("Total paginated results for pageSize = " + pageSize) - .isEqualTo(100); + .isEqualTo(20); } } finally { - for (int i = 0; i < 100; i++) { + for (int i = 0; i < 20; i++) { Namespace namespace = Namespace.of(prefix + i); restCatalog.dropNamespace(namespace); } } } + + @Test + public void testPaginatedListTables() { + String prefix = "testPaginatedListTables"; + Namespace namespace = Namespace.of(prefix); + restCatalog.createNamespace(namespace); + for (int i = 0; i < 20; i++) { + restCatalog.createTable(TableIdentifier.of(namespace, prefix + i), SCHEMA); + } + + try { + Assertions.assertThat(catalogApi.listTables(currentCatalogName, namespace)).hasSize(20); + for (var pageSize : List.of(1, 2, 3, 9, 10, 11, 19, 20, 21, 2000)) { + int total = 0; + String pageToken = null; + do { + ListTablesResponse response = + catalogApi.listTables( + currentCatalogName, namespace, pageToken, String.valueOf(pageSize)); + Assertions.assertThat(response.identifiers().size()).isLessThanOrEqualTo(pageSize); + total += response.identifiers().size(); + pageToken = response.nextPageToken(); + } while (pageToken != null); + Assertions.assertThat(total) + .as("Total paginated results for pageSize = " + pageSize) + .isEqualTo(20); + } + } finally { + for (int i = 0; i < 20; i++) { + restCatalog.dropTable(TableIdentifier.of(namespace, prefix + i)); + } + restCatalog.dropNamespace(namespace); + } + } } 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 27f876320c..fbc2fae697 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 @@ -52,8 +52,7 @@ void testReadEverythingPageToken() { Assertions.assertThat(PageToken.readEverything()).isEqualTo(PageToken.readEverything()); - Assertions - .assertThat(PageToken.readEverything().buildNextPage(List.of()).pageToken) + Assertions.assertThat(PageToken.readEverything().buildNextPage(List.of()).pageToken) .isInstanceOf(DonePageToken.class); } From 62914f93383dae2f323979e1a352a1c08d4251af Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 15 May 2025 21:16:54 -0700 Subject: [PATCH 15/18] changes per review --- .../PolarisEclipseLinkMetaStoreSessionImpl.java | 2 +- .../relational/jdbc/JdbcBasePersistenceImpl.java | 7 ++----- .../transactional/TreeMapTransactionalPersistenceImpl.java | 2 +- 3 files changed, 4 insertions(+), 7 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 b6bd237623..c51efcf8c3 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 @@ -481,7 +481,7 @@ public List lookupEntityActiveBatchInCurrentTxn( data = data.limit(hasPageSize.getPageSize()); } - return Page.fromItems(data.map(transformer).collect(Collectors.toList())); + return pageToken.buildNextPage(data.map(transformer).collect(Collectors.toList())); } /** {@inheritDoc} */ 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 8a7e2cf5a9..19fad08c29 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 @@ -440,11 +440,8 @@ public Page listEntities( } data.forEach(results::add); }); - List resultsOrEmpty = - results == null - ? Collections.emptyList() - : results.stream().map(transformer).collect(Collectors.toList()); - return Page.fromItems(resultsOrEmpty); + List resultsOrEmpty = results.stream().map(transformer).collect(Collectors.toList()); + return pageToken.buildNextPage(resultsOrEmpty); } catch (SQLException e) { throw new RuntimeException( String.format("Failed to retrieve polaris entities due to %s", e.getMessage()), e); 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 b5bc739d1b..7662041d54 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 @@ -376,7 +376,7 @@ public List lookupEntityActiveBatchInCurrentTxn( data = data.limit(((HasPageSize) pageToken).getPageSize()); } - return Page.fromItems(data.map(transformer).collect(Collectors.toList())); + return pageToken.buildNextPage(data.map(transformer).collect(Collectors.toList())); } /** {@inheritDoc} */ From c6611be0b1614da04d14b00402dd8b8929336fb7 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 22 May 2025 19:31:36 -0700 Subject: [PATCH 16/18] attempt pagerequest refactor --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 19 +++++-- .../jdbc/JdbcBasePersistenceImpl.java | 23 ++++---- .../AtomicOperationMetaStoreManager.java | 20 +++---- .../core/persistence/BasePersistence.java | 15 +++-- .../persistence/PolarisMetaStoreManager.java | 9 +-- .../TransactionWorkspaceMetaStoreManager.java | 6 +- .../pagination/EntityIdPageToken.java | 36 ++++++++++++ .../core/persistence/pagination/Page.java | 7 +++ .../persistence/pagination/PageRequest.java | 55 +++++++++++++++++++ .../AbstractTransactionalPersistence.java | 14 ++--- .../TransactionalMetaStoreManagerImpl.java | 35 +++++++----- .../TransactionalPersistence.java | 8 +-- .../TreeMapTransactionalPersistenceImpl.java | 17 ++++-- .../BasePolarisMetaStoreManagerTest.java | 18 +++--- .../PolarisTestMetaStoreManager.java | 16 +++--- .../quarkus/catalog/IcebergCatalogTest.java | 6 +- .../task/TableCleanupTaskHandlerTest.java | 12 ++-- .../service/admin/PolarisAdminService.java | 10 ++-- .../catalog/generic/GenericTableCatalog.java | 4 +- .../catalog/iceberg/IcebergCatalog.java | 39 ++++++------- .../service/catalog/policy/PolicyCatalog.java | 4 +- .../service/catalog/io/FileIOFactoryTest.java | 5 +- .../persistence/pagination/PageTokenTest.java | 2 +- 23 files changed, 254 insertions(+), 126 deletions(-) create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageRequest.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 c51efcf8c3..a77af8d7ee 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,8 +53,10 @@ 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.EntityIdPageToken; import org.apache.polaris.core.persistence.pagination.HasPageSize; import org.apache.polaris.core.persistence.pagination.Page; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.transactional.AbstractTransactionalPersistence; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; @@ -428,9 +430,9 @@ public List lookupEntityActiveBatchInCurrentTxn( long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { return this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageToken); + callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageRequest); } @Override @@ -440,7 +442,7 @@ public List lookupEntityActiveBatchInCurrentTxn( long parentId, @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { // full range scan under the parent for that type return this.listEntitiesInCurrentTxn( callCtx, @@ -456,7 +458,7 @@ public List lookupEntityActiveBatchInCurrentTxn( entity.getName(), entity.getTypeCode(), entity.getSubTypeCode()), - pageToken); + pageRequest); } @Override @@ -467,7 +469,9 @@ public List lookupEntityActiveBatchInCurrentTxn( @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, @Nonnull Function transformer, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { + PageToken pageToken = buildPageToken(pageRequest); + // full range scan under the parent for that type Stream data = this.store @@ -773,4 +777,9 @@ public void rollback() { session.getTransaction().rollback(); } } + + @Override + public PageToken buildPageToken(PageRequest pageRequest) { + return EntityIdPageToken.fromPageRequest(pageRequest); + } } 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 19fad08c29..bd7fc2af4c 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 @@ -53,8 +53,8 @@ import org.apache.polaris.core.persistence.pagination.EntityIdPageToken; import org.apache.polaris.core.persistence.pagination.HasPageSize; import org.apache.polaris.core.persistence.pagination.Page; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.pagination.PageToken; -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; @@ -365,7 +365,7 @@ public Page listEntities( long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { return listEntities( callCtx, catalogId, @@ -373,7 +373,7 @@ public Page listEntities( entityType, entity -> true, EntityNameLookupRecord::new, - pageToken); + pageRequest); } @Nonnull @@ -384,7 +384,7 @@ public Page listEntities( long parentId, @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { return listEntities( callCtx, catalogId, @@ -392,7 +392,7 @@ public Page listEntities( entityType, entityFilter, EntityNameLookupRecord::new, - pageToken); + pageRequest); } @Nonnull @@ -404,11 +404,7 @@ public Page listEntities( PolarisEntityType entityType, @Nonnull Predicate entityFilter, @Nonnull Function transformer, - @Nonnull PageToken pageToken) { - polarisDiagnostics.check( - (pageToken instanceof EntityIdPageToken || pageToken instanceof ReadEverythingPageToken), - "unrecognized_page_token"); - + @Nonnull PageRequest pageRequest) { Map params = Map.of( "catalog_id", @@ -423,7 +419,7 @@ public Page listEntities( // Limit can't be pushed down, due to client side filtering // absence of transaction. String query = QueryGenerator.generateSelectQuery(new ModelEntity(), params); - + PageToken pageToken = buildPageToken(pageRequest); if (pageToken instanceof EntityIdPageToken entityIdPageToken) { query += String.format(" AND id > %d ORDER BY id ASC", entityIdPageToken.getId()); } @@ -916,4 +912,9 @@ PolarisStorageIntegration loadPolarisStorageIntegration( private interface QueryAction { Integer apply(String query) throws SQLException; } + + @Override + public PageToken buildPageToken(PageRequest pageRequest) { + return EntityIdPageToken.fromPageRequest(pageRequest); + } } 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 2a32fb6f94..3c1aeba9bc 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 @@ -63,7 +63,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.pagination.Page; -import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyMappingUtil; @@ -690,7 +690,7 @@ private void revokeGrantRecord( @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, @Nonnull PolarisEntitySubType entitySubType, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { // get meta store we should be using BasePersistence ms = callCtx.getMetaStore(); @@ -703,15 +703,15 @@ private void revokeGrantRecord( ? 0l : catalogPath.get(catalogPath.size() - 1).getId(); Page resultPage = - ms.listEntities(callCtx, catalogId, parentId, entityType, pageToken); + ms.listEntities(callCtx, catalogId, parentId, entityType, pageRequest); // prune the returned list with only entities matching the entity subtype if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) { resultPage = - pageToken.buildNextPage( - resultPage.items.stream() - .filter(rec -> rec.getSubTypeCode() == entitySubType.getCode()) - .collect(Collectors.toList())); + resultPage.filter( + rec -> { + return rec.getSubTypeCode() == entitySubType.getCode(); + }); } // TODO: Use post-validation to enforce consistent view against catalogPath. In the @@ -1186,7 +1186,7 @@ private void revokeGrantRecord( PolarisEntityType.CATALOG_ROLE, entity -> true, Function.identity(), - PageToken.fromLimit(2)) + PageRequest.fromLimit(2)) .items; // if we have 2, we cannot drop the catalog. If only one left, better be the admin role @@ -1493,7 +1493,7 @@ private void revokeGrantRecord( @Override public @Nonnull EntitiesResult loadTasks( - @Nonnull PolarisCallContext callCtx, String executorId, PageToken pageToken) { + @Nonnull PolarisCallContext callCtx, String executorId, PageRequest pageRequest) { BasePersistence ms = callCtx.getMetaStore(); // find all available tasks @@ -1518,7 +1518,7 @@ private void revokeGrantRecord( || callCtx.getClock().millis() - taskState.lastAttemptStartTime > taskAgeTimeout; }, Function.identity(), - pageToken); + pageRequest); List loadedTasks = new ArrayList<>(); final AtomicInteger failedLeaseCount = new AtomicInteger(0); 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 dc85f2183b..00940fafd3 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.Page; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolicyMappingPersistence; @@ -272,7 +273,7 @@ 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 + * @param pageRequest the token to start listing after * @return the list of entities for the specified list operation */ @Nonnull @@ -281,7 +282,7 @@ Page listEntities( long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken); + @Nonnull PageRequest pageRequest); /** * List entities where some predicate returns true @@ -292,7 +293,7 @@ Page 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 + * @param pageRequest the token to start listing after * @return the list of entities for which the predicate returns true */ @Nonnull @@ -302,7 +303,7 @@ Page listEntities( long parentId, @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken); + @Nonnull PageRequest pageRequest); /** * List entities where some predicate returns true and transform the entities with a function @@ -315,6 +316,7 @@ Page listEntities( * returns true are returned in the list * @param transformer the transformation function applied to the {@link PolarisBaseEntity} before * returning + * @param pageRequest * @return the list of entities for which the predicate returns true */ @Nonnull @@ -325,7 +327,7 @@ Page listEntities( @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, @Nonnull Function transformer, - PageToken pageToken); + PageRequest pageRequest); /** * Lookup the current entityGrantRecordsVersion for the specified entity. That version is changed @@ -413,4 +415,7 @@ boolean hasChildren( default BasePersistence detach() { return this; } + + /** Construct a {@link PageToken} from a {@link PageRequest} */ + PageToken buildPageToken(PageRequest pageRequest); } 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 2a20ad5c1e..0d56d6af42 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,7 +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.persistence.pagination.PageRequest; import org.apache.polaris.core.policy.PolarisPolicyMappingManager; import org.apache.polaris.core.storage.PolarisCredentialVendor; @@ -114,6 +114,7 @@ EntityResult readEntityByName( * @param entityType entity type * @param entitySubType entity subtype. Can be the special value ANY_SUBTYPE to match any subtype. * Else exact match will be performed. + * @param pageRequest * @return all entities name, ids and subtype under the specified namespace. */ @Nonnull @@ -122,7 +123,7 @@ ListEntitiesResult listEntities( @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, @Nonnull PolarisEntitySubType entitySubType, - @Nonnull PageToken pageToken); + @Nonnull PageRequest pageRequest); /** * Generate a new unique id that can be used by the Polaris client when it needs to create a new @@ -302,12 +303,12 @@ EntityResult loadEntity( * * @param callCtx call context * @param executorId executor id - * @param pageToken page token to start after + * @param pageRequest page token to start after * @return list of tasks to be completed */ @Nonnull EntitiesResult loadTasks( - @Nonnull PolarisCallContext callCtx, String executorId, PageToken pageToken); + @Nonnull PolarisCallContext callCtx, String executorId, PageRequest pageRequest); /** * 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 b7ba47e83e..0b8ab7daf3 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 @@ -50,7 +50,7 @@ import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; -import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; @@ -120,7 +120,7 @@ public ListEntitiesResult listEntities( @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, @Nonnull PolarisEntitySubType entitySubType, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { callCtx.getDiagServices().fail("illegal_method_in_transaction_workspace", "listEntities"); return null; } @@ -322,7 +322,7 @@ public EntityResult loadEntity( @Override public EntitiesResult loadTasks( - @Nonnull PolarisCallContext callCtx, String executorId, PageToken pageToken) { + @Nonnull PolarisCallContext callCtx, String executorId, PageRequest pageRequest) { callCtx.getDiagServices().fail("illegal_method_in_transaction_workspace", "loadTasks"); return null; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java index 11a7d96b56..e8a1390517 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java @@ -19,10 +19,14 @@ package org.apache.polaris.core.persistence.pagination; import java.util.List; +import java.util.Optional; import org.apache.polaris.core.entity.EntityNameLookupRecord; import org.apache.polaris.core.entity.PolarisBaseEntity; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class EntityIdPageToken extends PageToken implements HasPageSize { + private static final Logger LOGGER = LoggerFactory.getLogger(EntityIdPageToken.class); public static final String PREFIX = "entity-id"; @@ -45,6 +49,38 @@ public EntityIdPageToken(long entityId, int pageSize) { this.pageSize = pageSize; } + /** + * Build an {@link EntityIdPageToken} from a {@link PageRequest}, or else a {@link + * ReadEverythingPageToken} if the request doesn't require pagination. + */ + public static PageToken fromPageRequest(PageRequest pageRequest) { + if (pageRequest.getPageTokenString().isEmpty()) { + if (pageRequest.getPageSize().isEmpty()) { + return PageToken.readEverything(); + } else { + return new EntityIdPageToken(pageRequest.getPageSize().get()); + } + } else { + String pageTokenString = pageRequest.getPageTokenString().get(); + Optional pageSize = pageRequest.getPageSize(); + + try { + String[] parts = pageRequest.getPageTokenString().get().split("/"); + if (parts.length < 1) { + throw new IllegalArgumentException("Invalid token format: " + pageTokenString); + } else if (parts[0].equals(EntityIdPageToken.PREFIX)) { + int resolvedPageSize = pageSize.orElse(Integer.parseInt(parts[2])); + return new EntityIdPageToken(Long.parseLong(parts[1]), resolvedPageSize); + } else { + throw new IllegalArgumentException("Unrecognized page token: " + pageTokenString); + } + } catch (NumberFormatException | IndexOutOfBoundsException e) { + LOGGER.debug(e.getMessage()); + throw new IllegalArgumentException("Invalid token format: " + pageTokenString); + } + } + } + public long getId() { return entityId; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/Page.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/Page.java index 18287f85c1..b3714af679 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/Page.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/Page.java @@ -19,6 +19,8 @@ package org.apache.polaris.core.persistence.pagination; import java.util.List; +import java.util.function.Predicate; +import java.util.stream.Collectors; /** * An immutable page of items plus their paging cursor. The {@link PageToken} here can be used to @@ -39,4 +41,9 @@ public Page(PageToken pageToken, List items) { public static Page fromItems(List items) { return new Page<>(new DonePageToken(), items); } + + public Page filter(Predicate predicate) { + return new Page<>( + this.pageToken, this.items.stream().filter(predicate).collect(Collectors.toList())); + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageRequest.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageRequest.java new file mode 100644 index 0000000000..1a4b1d256d --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageRequest.java @@ -0,0 +1,55 @@ +/* + * 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.Optional; + +/** + * A wrapper for pagination information passed in as part of a request. This can potentially be + * translated into a `PageToken` + */ +public class PageRequest { + private final Optional pageTokenStringOpt; + private final Optional pageSizeOpt; + + public PageRequest(String pageTokenString, Integer pageSize) { + this.pageTokenStringOpt = Optional.ofNullable(pageTokenString); + this.pageSizeOpt = Optional.ofNullable(pageSize); + } + + public static PageRequest readEverything() { + return new PageRequest(null, null); + } + + public static PageRequest fromLimit(Integer pageSize) { + return new PageRequest(null, pageSize); + } + + public boolean isPaginationRequested() { + return pageTokenStringOpt.isPresent() || pageSizeOpt.isPresent(); + } + + public Optional getPageTokenString() { + return pageTokenStringOpt; + } + + public Optional getPageSize() { + return pageSizeOpt; + } +} 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 e63ea6fedb..7070657bf1 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 @@ -38,7 +38,7 @@ import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; import org.apache.polaris.core.persistence.pagination.Page; -import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; @@ -359,10 +359,10 @@ public Page listEntities( long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { return runInReadTransaction( callCtx, - () -> this.listEntitiesInCurrentTxn(callCtx, catalogId, parentId, entityType, pageToken)); + () -> this.listEntitiesInCurrentTxn(callCtx, catalogId, parentId, entityType, pageRequest)); } /** {@inheritDoc} */ @@ -374,12 +374,12 @@ public Page listEntities( long parentId, @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { return runInReadTransaction( callCtx, () -> this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, entityFilter, pageToken)); + callCtx, catalogId, parentId, entityType, entityFilter, pageRequest)); } /** {@inheritDoc} */ @@ -392,12 +392,12 @@ public Page listEntities( @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, @Nonnull Function transformer, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { return runInReadTransaction( callCtx, () -> this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, entityFilter, transformer, pageToken)); + callCtx, catalogId, parentId, entityType, entityFilter, transformer, pageRequest)); } /** {@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 62f526a6db..df799fc4a5 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 @@ -65,6 +65,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.pagination.Page; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.policy.PolicyEntity; @@ -680,8 +681,8 @@ private void bootstrapPolarisService( } /** - * See {@link #listEntities(PolarisCallContext, List, PolarisEntityType, PolarisEntitySubType, - * PageToken)} + * See {@link PolarisMetaStoreManager#listEntities(PolarisCallContext, List, PolarisEntityType, + * PolarisEntitySubType, PageRequest)} */ private @Nonnull ListEntitiesResult listEntities( @Nonnull PolarisCallContext callCtx, @@ -689,7 +690,7 @@ private void bootstrapPolarisService( @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, @Nonnull PolarisEntitySubType entitySubType, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { // first resolve again the catalogPath to that entity PolarisEntityResolver resolver = new PolarisEntityResolver(callCtx, ms, catalogPath); @@ -702,15 +703,19 @@ private void bootstrapPolarisService( // return list of active entities Page resultPage = ms.listEntitiesInCurrentTxn( - callCtx, resolver.getCatalogIdOrNull(), resolver.getParentId(), entityType, pageToken); + callCtx, + resolver.getCatalogIdOrNull(), + resolver.getParentId(), + entityType, + pageRequest); // prune the returned list with only entities matching the entity subtype if (entitySubType != PolarisEntitySubType.ANY_SUBTYPE) { resultPage = - pageToken.buildNextPage( - resultPage.items.stream() - .filter(rec -> rec.getSubTypeCode() == entitySubType.getCode()) - .collect(Collectors.toList())); + resultPage.filter( + rec -> { + return rec.getSubTypeCode() == entitySubType.getCode(); + }); } // done @@ -724,14 +729,14 @@ private void bootstrapPolarisService( @Nullable List catalogPath, @Nonnull PolarisEntityType entityType, @Nonnull PolarisEntitySubType entitySubType, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { // 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, pageToken)); + () -> listEntities(callCtx, ms, catalogPath, entityType, entitySubType, pageRequest)); } /** {@link #createPrincipal(PolarisCallContext, PolarisBaseEntity)} */ @@ -1374,7 +1379,7 @@ private void bootstrapPolarisService( PolarisEntityType.CATALOG_ROLE, entity -> true, Function.identity(), - PageToken.fromLimit(2)) + PageRequest.fromLimit(2)) .items; // if we have 2, we cannot drop the catalog. If only one left, better be the admin role @@ -1934,7 +1939,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( @Nonnull PolarisCallContext callCtx, @Nonnull TransactionalPersistence ms, String executorId, - PageToken pageToken) { + PageRequest pageRequest) { // find all available tasks Page availableTasks = @@ -1958,7 +1963,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( || callCtx.getClock().millis() - taskState.lastAttemptStartTime > taskAgeTimeout; }, Function.identity(), - pageToken); + pageRequest); List loadedTasks = new ArrayList<>(); availableTasks.items.forEach( @@ -1997,9 +2002,9 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( @Override public @Nonnull EntitiesResult loadTasks( - @Nonnull PolarisCallContext callCtx, String executorId, PageToken pageToken) { + @Nonnull PolarisCallContext callCtx, String executorId, PageRequest pageRequest) { TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); - return ms.runInTransaction(callCtx, () -> this.loadTasks(callCtx, ms, executorId, pageToken)); + return ms.runInTransaction(callCtx, () -> this.loadTasks(callCtx, ms, executorId, pageRequest)); } /** {@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 1c58334d55..6189101416 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 @@ -37,7 +37,7 @@ import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.IntegrationPersistence; import org.apache.polaris.core.persistence.pagination.Page; -import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.policy.TransactionalPolicyMappingPersistence; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -208,7 +208,7 @@ Page listEntitiesInCurrentTxn( long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken); + @Nonnull PageRequest pageRequest); /** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */ @Nonnull @@ -218,7 +218,7 @@ Page listEntitiesInCurrentTxn( long parentId, @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken); + @Nonnull PageRequest pageRequest); /** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */ @Nonnull @@ -229,7 +229,7 @@ Page listEntitiesInCurrentTxn( @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, @Nonnull Function transformer, - @Nonnull PageToken pageToken); + @Nonnull PageRequest pageRequest); /** * 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 7662041d54..c617f4f69a 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 @@ -43,6 +43,7 @@ import org.apache.polaris.core.persistence.pagination.EntityIdPageToken; import org.apache.polaris.core.persistence.pagination.HasPageSize; import org.apache.polaris.core.persistence.pagination.Page; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; @@ -312,9 +313,9 @@ public List lookupEntityActiveBatchInCurrentTxn( long catalogId, long parentId, @Nonnull PolarisEntityType entityType, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { return this.listEntitiesInCurrentTxn( - callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageToken); + callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue(), pageRequest); } @Override @@ -324,7 +325,7 @@ public List lookupEntityActiveBatchInCurrentTxn( long parentId, @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { // full range scan under the parent for that type return this.listEntitiesInCurrentTxn( callCtx, @@ -340,7 +341,7 @@ public List lookupEntityActiveBatchInCurrentTxn( entity.getName(), entity.getTypeCode(), entity.getSubTypeCode()), - pageToken); + pageRequest); } @Override @@ -351,7 +352,7 @@ public List lookupEntityActiveBatchInCurrentTxn( @Nonnull PolarisEntityType entityType, @Nonnull Predicate entityFilter, @Nonnull Function transformer, - @Nonnull PageToken pageToken) { + @Nonnull PageRequest pageRequest) { // full range scan under the parent for that type Stream data = this.store @@ -364,6 +365,7 @@ public List lookupEntityActiveBatchInCurrentTxn( this.lookupEntityInCurrentTxn( callCtx, catalogId, nameRecord.getId(), entityType.getCode())); + PageToken pageToken = buildPageToken(pageRequest); if (pageToken instanceof EntityIdPageToken) { data = data.sorted(Comparator.comparingLong(PolarisEntityCore::getId)) @@ -659,4 +661,9 @@ record -> this.store.getSlicePolicyMappingRecordsByPolicy().delete(record)); .getSlicePolicyMappingRecordsByPolicy() .readRange(this.store.buildPrefixKeyComposite(policyCatalogId, policyId)); } + + @Override + public PageToken buildPageToken(PageRequest pageRequest) { + return EntityIdPageToken.fromPageRequest(pageRequest); + } } 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 0f834bc760..0dac7518b2 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,7 +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.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.assertj.core.api.Assertions; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.BeforeEach; @@ -130,7 +130,7 @@ protected void testCreateEntities() { null, PolarisEntityType.TASK, PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities(); Assertions.assertThat(listedEntities) .isNotNull() @@ -309,7 +309,7 @@ protected void testLoadTasks() { PolarisMetaStoreManager metaStoreManager = polarisTestMetaStoreManager.polarisMetaStoreManager; PolarisCallContext callCtx = polarisTestMetaStoreManager.polarisCallContext; List taskList = - metaStoreManager.loadTasks(callCtx, executorId, PageToken.fromLimit(5)).getEntities(); + metaStoreManager.loadTasks(callCtx, executorId, PageRequest.fromLimit(5)).getEntities(); Assertions.assertThat(taskList) .isNotNull() .isNotEmpty() @@ -329,7 +329,7 @@ protected 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, PageToken.fromLimit(5)).getEntities(); + metaStoreManager.loadTasks(callCtx, executorId, PageRequest.fromLimit(5)).getEntities(); Assertions.assertThat(newTaskList) .isNotNull() .isNotEmpty() @@ -343,7 +343,7 @@ protected void testLoadTasks() { // only 10 tasks are unassigned. Requesting 20, we should only receive those 10 List lastTen = - metaStoreManager.loadTasks(callCtx, executorId, PageToken.fromLimit(20)).getEntities(); + metaStoreManager.loadTasks(callCtx, executorId, PageRequest.fromLimit(20)).getEntities(); Assertions.assertThat(lastTen) .isNotNull() @@ -357,7 +357,7 @@ protected void testLoadTasks() { .collect(Collectors.toSet()); List emtpyList = - metaStoreManager.loadTasks(callCtx, executorId, PageToken.fromLimit(20)).getEntities(); + metaStoreManager.loadTasks(callCtx, executorId, PageRequest.fromLimit(20)).getEntities(); Assertions.assertThat(emtpyList).isNotNull().isEmpty(); @@ -365,7 +365,7 @@ protected void testLoadTasks() { // all the tasks are unassigned. Fetch them all List allTasks = - metaStoreManager.loadTasks(callCtx, executorId, PageToken.fromLimit(20)).getEntities(); + metaStoreManager.loadTasks(callCtx, executorId, PageRequest.fromLimit(20)).getEntities(); Assertions.assertThat(allTasks) .isNotNull() @@ -380,7 +380,7 @@ protected void testLoadTasks() { timeSource.add(Duration.ofMinutes(10)); List finalList = - metaStoreManager.loadTasks(callCtx, executorId, PageToken.fromLimit(20)).getEntities(); + metaStoreManager.loadTasks(callCtx, executorId, PageRequest.fromLimit(20)).getEntities(); Assertions.assertThat(finalList).isNotNull().isEmpty(); } @@ -410,7 +410,7 @@ protected void testLoadTasksInParallel() throws Exception { try { taskList = metaStoreManager - .loadTasks(callCtx, executorId, PageToken.fromLimit(5)) + .loadTasks(callCtx, executorId, PageRequest.fromLimit(5)) .getEntities(); taskList.stream().map(PolarisBaseEntity::getName).forEach(taskNames::add); } catch (RetryOnConcurrencyException e) { 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 8cddecae3e..550e23888a 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,7 +51,7 @@ 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; -import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; @@ -768,7 +768,7 @@ void dropEntity(List catalogPath, PolarisBaseEntity entityToD path, PolarisEntityType.NAMESPACE, PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities(); Assertions.assertThat(children).isNotNull(); if (children.isEmpty() && entity.getType() == PolarisEntityType.NAMESPACE) { @@ -779,7 +779,7 @@ void dropEntity(List catalogPath, PolarisBaseEntity entityToD path, PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities(); Assertions.assertThat(children).isNotNull(); } else if (children.isEmpty()) { @@ -790,7 +790,7 @@ void dropEntity(List catalogPath, PolarisBaseEntity entityToD path, PolarisEntityType.CATALOG_ROLE, PolarisEntitySubType.ANY_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities(); Assertions.assertThat(children).isNotNull(); // if only one left, it can be dropped. @@ -1564,7 +1564,7 @@ private void validateListReturn( path, entityType, entitySubType, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities(); Assertions.assertThat(result).isNotNull(); @@ -1882,7 +1882,7 @@ void validateBootstrap() { null, PolarisEntityType.PRINCIPAL, PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities(); // ensure not null, one element only @@ -1909,7 +1909,7 @@ void validateBootstrap() { null, PolarisEntityType.PRINCIPAL_ROLE, PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities(); // ensure not null, one element only @@ -2648,7 +2648,7 @@ public void testLookup() { null, PolarisEntityType.PRINCIPAL, PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .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 12e8d558ae..8e5658642b 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 @@ -99,7 +99,7 @@ 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.Page; -import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.transactional.TransactionalPersistence; import org.apache.polaris.core.secrets.UserSecretsManager; import org.apache.polaris.core.secrets.UserSecretsManagerFactory; @@ -1551,7 +1551,7 @@ public void testDropTableWithPurge() { .rejects(TABLE); List tasks = metaStoreManager - .loadTasks(polarisContext, "testExecutor", PageToken.fromLimit(1)) + .loadTasks(polarisContext, "testExecutor", PageRequest.fromLimit(1)) .getEntities(); Assertions.assertThat(tasks).hasSize(1); TaskEntity taskEntity = TaskEntity.of(tasks.get(0)); @@ -1762,7 +1762,7 @@ public void testFileIOWrapper() { TaskEntity.of( metaStoreManager .loadTasks( - callContext.getPolarisCallContext(), "testExecutor", PageToken.fromLimit(1)) + callContext.getPolarisCallContext(), "testExecutor", PageRequest.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 5e39028c92..d6d7930f31 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 @@ -55,7 +55,7 @@ import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; -import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.service.catalog.io.FileIOFactory; import org.apache.polaris.service.task.BatchFileCleanupTaskHandler; @@ -155,7 +155,7 @@ public void testTableCleanup() throws IOException { assertThat( metaStoreManagerFactory .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(2)) + .loadTasks(callContext.getPolarisCallContext(), "test", PageRequest.fromLimit(2)) .getEntities()) .hasSize(2) .satisfiesExactlyInAnyOrder( @@ -235,7 +235,7 @@ public void close() { assertThat( metaStoreManagerFactory .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(5)) + .loadTasks(callContext.getPolarisCallContext(), "test", PageRequest.fromLimit(5)) .getEntities()) .hasSize(2); } @@ -296,7 +296,7 @@ public void close() { assertThat( metaStoreManagerFactory .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(5)) + .loadTasks(callContext.getPolarisCallContext(), "test", PageRequest.fromLimit(5)) .getEntities()) .hasSize(4) .satisfiesExactlyInAnyOrder( @@ -416,7 +416,7 @@ public void testTableCleanupMultipleSnapshots() throws IOException { List entities = metaStoreManagerFactory .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(5)) + .loadTasks(callContext.getPolarisCallContext(), "test", PageRequest.fromLimit(5)) .getEntities(); List manifestCleanupTasks = @@ -590,7 +590,7 @@ public void testTableCleanupMultipleMetadata() throws IOException { List entities = metaStoreManagerFactory .getOrCreateMetaStoreManager(callContext.getRealmContext()) - .loadTasks(callContext.getPolarisCallContext(), "test", PageToken.fromLimit(6)) + .loadTasks(callContext.getPolarisCallContext(), "test", PageRequest.fromLimit(6)) .getEntities(); List manifestCleanupTasks = 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 c51c9a8ebc..1c73cca843 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 @@ -95,7 +95,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.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; @@ -895,7 +895,7 @@ private List listCatalogsUnsafe() { null, PolarisEntityType.CATALOG, PolarisEntitySubType.ANY_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities() .stream() .map( @@ -1065,7 +1065,7 @@ public List listPrincipals() { null, PolarisEntityType.PRINCIPAL, PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities() .stream() .map( @@ -1178,7 +1178,7 @@ public List listPrincipalRoles() { null, PolarisEntityType.PRINCIPAL_ROLE, PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities() .stream() .map( @@ -1310,7 +1310,7 @@ public List listCatalogRoles(String catalogName) { PolarisEntity.toCoreList(List.of(catalogEntity)), PolarisEntityType.CATALOG_ROLE, PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .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 79adeaee85..525a0ddc02 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 @@ -37,7 +37,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.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -177,7 +177,7 @@ public List listGenericTables(Namespace namespace) { PolarisEntity.toCoreList(catalogPath), PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.GENERIC_TABLE, - PageToken.readEverything()) + PageRequest.readEverything()) .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 3b36a38490..1e05cffb5e 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 @@ -110,6 +110,7 @@ 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.Page; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; @@ -502,20 +503,20 @@ public boolean dropTable(TableIdentifier tableIdentifier, boolean purge) { @Override public List listTables(Namespace namespace) { - return listTables(namespace, PageToken.readEverything()).items; + return listTables(namespace, PageRequest.readEverything()).items; } public Page listTables(Namespace namespace, String pageToken, Integer pageSize) { - return listTables(namespace, buildPageToken(pageToken, pageSize)); + return listTables(namespace, buildPageRequest(pageToken, pageSize)); } - private Page listTables(Namespace namespace, PageToken pageToken) { + private Page listTables(Namespace namespace, PageRequest pageRequest) { if (!namespaceExists(namespace)) { throw new NoSuchNamespaceException( "Cannot list tables for namespace. Namespace does not exist: '%s'", namespace); } - return listTableLike(PolarisEntitySubType.ICEBERG_TABLE, namespace, pageToken); + return listTableLike(PolarisEntitySubType.ICEBERG_TABLE, namespace, pageRequest); } @Override @@ -825,14 +826,14 @@ public List listNamespaces() { @Override public List listNamespaces(Namespace namespace) throws NoSuchNamespaceException { - return listNamespaces(namespace, PageToken.readEverything()).items; + return listNamespaces(namespace, PageRequest.readEverything()).items; } public Page listNamespaces(Namespace namespace, String pageToken, Integer pageSize) { - return listNamespaces(namespace, buildPageToken(pageToken, pageSize)); + return listNamespaces(namespace, buildPageRequest(pageToken, pageSize)); } - private Page listNamespaces(Namespace namespace, PageToken pageToken) + private Page listNamespaces(Namespace namespace, PageRequest pageRequest) throws NoSuchNamespaceException { PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); if (resolvedEntities == null) { @@ -847,7 +848,7 @@ private Page listNamespaces(Namespace namespace, PageToken pageToken) PolarisEntity.toCoreList(catalogPath), PolarisEntityType.NAMESPACE, PolarisEntitySubType.NULL_SUBTYPE, - pageToken); + pageRequest); List entities = PolarisEntity.toNameAndIdList(listResult.getEntities()); List namespaces = PolarisCatalogHelpers.nameAndIdToNamespaces(catalogPath, entities); @@ -866,20 +867,20 @@ public void close() throws IOException { @Override public List listViews(Namespace namespace) { - return listViews(namespace, PageToken.readEverything()).items; + return listViews(namespace, PageRequest.readEverything()).items; } public Page listViews(Namespace namespace, String pageToken, Integer pageSize) { - return listViews(namespace, buildPageToken(pageToken, pageSize)); + return listViews(namespace, buildPageRequest(pageToken, pageSize)); } - private Page listViews(Namespace namespace, PageToken pageToken) { + private Page listViews(Namespace namespace, PageRequest pageRequest) { if (!namespaceExists(namespace)) { throw new NoSuchNamespaceException( "Cannot list views for namespace. Namespace does not exist: '%s'", namespace); } - return listTableLike(PolarisEntitySubType.ICEBERG_VIEW, namespace, pageToken); + return listTableLike(PolarisEntitySubType.ICEBERG_VIEW, namespace, pageRequest); } @Override @@ -1107,7 +1108,7 @@ private void validateNoLocationOverlap( parentPath.stream().map(PolarisEntity::toCore).collect(Collectors.toList()), PolarisEntityType.NAMESPACE, PolarisEntitySubType.ANY_SUBTYPE, - PageToken.readEverything()); + PageRequest.readEverything()); if (!siblingNamespacesResult.isSuccess()) { throw new IllegalStateException( "Unable to resolve siblings entities to validate location - could not list namespaces"); @@ -1133,7 +1134,7 @@ private void validateNoLocationOverlap( .collect(Collectors.toList()), PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.ANY_SUBTYPE, - PageToken.readEverything()); + PageRequest.readEverything()); if (!siblingTablesResult.isSuccess()) { throw new IllegalStateException( "Unable to resolve siblings entities to validate location - could not list tables"); @@ -2499,7 +2500,7 @@ private void createNonExistingNamespaces(Namespace namespace) { } private Page listTableLike( - PolarisEntitySubType subType, Namespace namespace, PageToken pageToken) { + PolarisEntitySubType subType, Namespace namespace, PageRequest pageRequest) { PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace); if (resolvedEntities == null) { // Illegal state because the namespace should've already been in the static resolution set. @@ -2515,7 +2516,7 @@ private Page listTableLike( PolarisEntity.toCoreList(catalogPath), PolarisEntityType.TABLE_LIKE, subType, - pageToken); + pageRequest); List entities = PolarisEntity.toNameAndIdList(listResult.getEntities()); List identifiers = @@ -2574,7 +2575,7 @@ private int getMaxMetadataRefreshRetries() { } /** Build a {@link PageToken} from a string and page size. */ - private PageToken buildPageToken(@Nullable String tokenString, @Nullable Integer pageSize) { + private PageRequest buildPageRequest(@Nullable String tokenString, @Nullable Integer pageSize) { boolean paginationEnabled = callContext .getPolarisCallContext() @@ -2584,9 +2585,9 @@ private PageToken buildPageToken(@Nullable String tokenString, @Nullable Integer catalogEntity, FeatureConfiguration.LIST_PAGINATION_ENABLED); if (!paginationEnabled) { - return PageToken.readEverything(); + return PageRequest.readEverything(); } else { - return PageToken.build(tokenString, pageSize); + return new PageRequest(tokenString, pageSize); } } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java index e0edebfc64..3cc0847194 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java @@ -50,7 +50,7 @@ import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult; -import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.apache.polaris.core.policy.PolicyEntity; import org.apache.polaris.core.policy.PolicyType; @@ -169,7 +169,7 @@ public List listPolicies(Namespace namespace, PolicyType polic PolarisEntity.toCoreList(catalogPath), PolarisEntityType.POLICY, PolarisEntitySubType.NULL_SUBTYPE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities() .stream() .map( 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 30afc5c09a..1e27e7ca4f 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 @@ -46,7 +46,7 @@ import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.*; -import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.service.TestServices; import org.apache.polaris.service.catalog.PolarisPassthroughResolutionView; import org.apache.polaris.service.catalog.iceberg.IcebergCatalog; @@ -185,7 +185,8 @@ public void testLoadFileIOForCleanupTask() { testServices .metaStoreManagerFactory() .getOrCreateMetaStoreManager(realmContext) - .loadTasks(callContext.getPolarisCallContext(), "testExecutor", PageToken.fromLimit(1)) + .loadTasks( + callContext.getPolarisCallContext(), "testExecutor", PageRequest.fromLimit(1)) .getEntities(); Assertions.assertThat(tasks).hasSize(1); TaskEntity taskEntity = TaskEntity.of(tasks.get(0)); 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 fbc2fae697..e863f94cb3 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 @@ -92,7 +92,7 @@ void testInvalidPageTokens() { .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Unrecognized page token"); - PageToken goodToken = PageToken.fromLimit(100); + PageToken goodToken = new EntityIdPageToken(100); Assertions.assertThatCode(() -> PageToken.fromString(goodToken.toTokenString() + "???")) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Invalid token format"); From aa90da2288cafcea0d7c3595a961aa14b461bf0a Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 22 May 2025 19:37:23 -0700 Subject: [PATCH 17/18] stable --- .../service/catalog/generic/PolarisGenericTableCatalog.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java index 2b884e787f..3a8bc143bb 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java @@ -37,7 +37,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.PageToken; +import org.apache.polaris.core.persistence.pagination.PageRequest; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -191,7 +191,7 @@ public List listGenericTables(Namespace namespace) { PolarisEntity.toCoreList(catalogPath), PolarisEntityType.TABLE_LIKE, PolarisEntitySubType.GENERIC_TABLE, - PageToken.readEverything()) + PageRequest.readEverything()) .getEntities()); return PolarisCatalogHelpers.nameAndIdToTableIdentifiers(catalogPath, entities); } From 95f841050a13f5f66ee120edb6c3dd69987eb81a Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 28 May 2025 09:36:55 -0700 Subject: [PATCH 18/18] stable --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 3 +- .../jdbc/JdbcBasePersistenceImpl.java | 3 +- .../core/persistence/BasePersistence.java | 4 -- .../pagination/EntityIdPageToken.java | 2 + .../pagination/LimitPageToken.java | 47 +++++++++++++++++++ .../persistence/pagination/PageRequest.java | 2 +- .../persistence/pagination/PageToken.java | 5 -- .../TreeMapTransactionalPersistenceImpl.java | 3 +- 8 files changed, 53 insertions(+), 16 deletions(-) create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/LimitPageToken.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 8f6f9b8934..e891ec4905 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 @@ -783,8 +783,7 @@ public void rollback() { } } - @Override - public PageToken buildPageToken(PageRequest pageRequest) { + private PageToken buildPageToken(PageRequest pageRequest) { return EntityIdPageToken.fromPageRequest(pageRequest); } } 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 9ba49cedd4..c99b8e41c4 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 @@ -924,8 +924,7 @@ private interface QueryAction { Integer apply(String query) throws SQLException; } - @Override - public PageToken buildPageToken(PageRequest pageRequest) { + private PageToken buildPageToken(PageRequest pageRequest) { return EntityIdPageToken.fromPageRequest(pageRequest); } } 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 00940fafd3..6394fe7574 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 @@ -33,7 +33,6 @@ import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.persistence.pagination.Page; import org.apache.polaris.core.persistence.pagination.PageRequest; -import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.policy.PolicyMappingPersistence; /** @@ -415,7 +414,4 @@ boolean hasChildren( default BasePersistence detach() { return this; } - - /** Construct a {@link PageToken} from a {@link PageRequest} */ - PageToken buildPageToken(PageRequest pageRequest); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java index e8a1390517..dfa1df10f9 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/EntityIdPageToken.java @@ -71,6 +71,8 @@ public static PageToken fromPageRequest(PageRequest pageRequest) { } else if (parts[0].equals(EntityIdPageToken.PREFIX)) { int resolvedPageSize = pageSize.orElse(Integer.parseInt(parts[2])); return new EntityIdPageToken(Long.parseLong(parts[1]), resolvedPageSize); + } else if (parts[0].equals(LimitPageToken.PREFIX)) { + return new LimitPageToken(pageRequest.getPageSize().get()); } else { throw new IllegalArgumentException("Unrecognized page token: " + pageTokenString); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/LimitPageToken.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/LimitPageToken.java new file mode 100644 index 0000000000..e3f28b171a --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/LimitPageToken.java @@ -0,0 +1,47 @@ +/* + * 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; + +public class LimitPageToken extends PageToken implements HasPageSize { + public static final String PREFIX = "limit"; + + private final int pageSize; + + public LimitPageToken(int pageSize) { + this.pageSize = pageSize; + } + + @Override + public int getPageSize() { + return pageSize; + } + + @Override + public String toTokenString() { + return null; + } + + @Override + protected PageToken updated(List newData) { + return new DonePageToken(); + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageRequest.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageRequest.java index 1a4b1d256d..f13e74a763 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageRequest.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageRequest.java @@ -38,7 +38,7 @@ public static PageRequest readEverything() { } public static PageRequest fromLimit(Integer pageSize) { - return new PageRequest(null, pageSize); + return new PageRequest(LimitPageToken.PREFIX, pageSize); } public boolean isPaginationRequested() { 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 index 90bb811f90..c1be8707d2 100644 --- 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 @@ -45,11 +45,6 @@ public static PageToken fromString(String token) { return build(token, null); } - /** Build a new PageToken from a limit */ - public static PageToken fromLimit(Integer pageSize) { - return build(null, pageSize); - } - /** Build a {@link PageToken} from the input string and page size */ public static PageToken build(String token, Integer pageSize) { if (token == null || token.isEmpty()) { 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 c5fba19ef5..1e9a56448b 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 @@ -676,8 +676,7 @@ record -> this.store.getSlicePolicyMappingRecordsByPolicy().delete(record)); .readRange(this.store.buildPrefixKeyComposite(policyTypeCode, policyCatalogId, policyId)); } - @Override - public PageToken buildPageToken(PageRequest pageRequest) { + private PageToken buildPageToken(PageRequest pageRequest) { return EntityIdPageToken.fromPageRequest(pageRequest); } }