This is an automated email from the ASF dual-hosted git repository. dimas pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push: new a0a2b87a9 feat: enforce LIST_PAGINATION_ENABLED (#2401) a0a2b87a9 is described below commit a0a2b87a9fac7eadbc6c0583c905685f5e0932f4 Author: Dmitri Bourlatchkov <dmitri.bourlatch...@gmail.com> AuthorDate: Wed Aug 20 14:19:01 2025 -0400 feat: enforce LIST_PAGINATION_ENABLED (#2401) * feat: enforce LIST_PAGINATION_ENABLED The enforcement of the LIST_PAGINATION_ENABLED flag was missed in #1938. This change make the flag effective as discussed in #2296. Note: this causes a change in the default Polaris behaviour (no pagination by default) with respect to the previous state of `main`. However, there is no behaviour change with respect to 1.0.0 or 1.0.1 as previous releases did not have #1938. --- CHANGELOG.md | 3 ++ .../apache/polaris/service/it/env/CatalogApi.java | 14 +++++++ .../it/test/PolarisRestCatalogIntegrationBase.java | 48 ++++++++++++++++++++++ .../core/persistence/pagination/PageToken.java | 8 +++- .../core/persistence/pagination/PageTokenUtil.java | 9 ++-- .../core/persistence/pagination/PageTokenTest.java | 18 ++++---- .../catalog/iceberg/IcebergCatalogHandler.java | 12 ++++-- .../catalog/AbstractIcebergCatalogTest.java | 2 +- 8 files changed, 97 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d378d847..c020e816b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,9 @@ the authentication parameters are picked from the environment or configuration f - The `DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED` feature was added to support placing tables at locations that better optimize for object storage. +- The `LIST_PAGINATION_ENABLED` (default: false) feature flag can be used to enable pagination + in the Iceberg REST Catalog API. + - The Helm chart now supports Pod Disruption Budgets (PDBs) for Polaris components. This allows users to define the minimum number of pods that must be available during voluntary disruptions, such as node maintenance. 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 eb400b1e6..d06e44209 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 @@ -205,6 +205,20 @@ public class CatalogApi extends PolarisRestApi { } } + public ListTablesResponse listViews( + String catalog, Namespace namespace, String pageToken, String pageSize) { + String ns = RESTUtil.encodeNamespace(namespace); + Map<String, String> queryParams = new HashMap<>(); + queryParams.put("pageToken", pageToken); + queryParams.put("pageSize", pageSize); + try (Response res = + request("v1/{cat}/namespaces/" + ns + "/views", Map.of("cat", catalog), queryParams) + .get()) { + assertThat(res.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + return res.readEntity(ListTablesResponse.class); + } + } + public void dropView(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/PolarisRestCatalogIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java index 4a4ab6bbf..7a1889b93 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java @@ -2160,4 +2160,52 @@ public abstract class PolarisRestCatalogIntegrationBase extends CatalogTests<RES restCatalog.dropNamespace(namespace); } } + + @Test + public void testNonPaginatedListTablesViewNamespaces() { + Catalog catalog = managementApi.getCatalog(currentCatalogName); + Map<String, String> catalogProps = new HashMap<>(catalog.getProperties().toMap()); + catalogProps.put(FeatureConfiguration.LIST_PAGINATION_ENABLED.catalogConfig(), "false"); + managementApi.updateCatalog(catalog, catalogProps); + + String prefix = "testNonPaginatedListTablesViewNamespaces"; + Namespace namespace = Namespace.of(prefix); + restCatalog.createNamespace(namespace); + for (int i = 0; i < 5; i++) { + restCatalog.createNamespace(Namespace.of(prefix, "nested-ns" + i)); + restCatalog.createTable(TableIdentifier.of(namespace, "table" + i), SCHEMA); + restCatalog + .buildView(TableIdentifier.of(namespace, "view" + i)) + .withSchema(SCHEMA) + .withDefaultNamespace(namespace) + .withQuery("spark", VIEW_QUERY) + .create(); + } + + assertThat(catalogApi.listTables(currentCatalogName, namespace)).hasSize(5); + // Note: no pagination per feature config + ListTablesResponse response = catalogApi.listTables(currentCatalogName, namespace, null, "2"); + assertThat(response.identifiers()).hasSize(5); + assertThat(response.nextPageToken()).isNull(); + response = catalogApi.listTables(currentCatalogName, namespace, "fake-token", null); + assertThat(response.identifiers()).hasSize(5); + assertThat(response.nextPageToken()).isNull(); + + assertThat(catalogApi.listViews(currentCatalogName, namespace)).hasSize(5); + response = catalogApi.listViews(currentCatalogName, namespace, null, "2"); + assertThat(response.identifiers()).hasSize(5); + assertThat(response.nextPageToken()).isNull(); + response = catalogApi.listViews(currentCatalogName, namespace, "fake-token", null); + assertThat(response.identifiers()).hasSize(5); + assertThat(response.nextPageToken()).isNull(); + + assertThat(catalogApi.listNamespaces(currentCatalogName, namespace)).hasSize(5); + ListNamespacesResponse nsResponse = + catalogApi.listNamespaces(currentCatalogName, namespace, null, "2"); + assertThat(nsResponse.namespaces()).hasSize(5); + assertThat(nsResponse.nextPageToken()).isNull(); + nsResponse = catalogApi.listNamespaces(currentCatalogName, namespace, "fake-token", null); + assertThat(nsResponse.namespaces()).hasSize(5); + assertThat(nsResponse.nextPageToken()).isNull(); + } } 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 a1dceffdd..a48cbe1bd 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 @@ -24,6 +24,7 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize; import jakarta.annotation.Nullable; import java.util.Optional; import java.util.OptionalInt; +import java.util.function.BooleanSupplier; import org.apache.polaris.immutables.PolarisImmutable; /** A wrapper for pagination information passed in as part of a request. */ @@ -86,7 +87,10 @@ public interface PageToken { * @see Page#encodedResponseToken() */ static PageToken build( - @Nullable String serializedPageToken, @Nullable Integer requestedPageSize) { - return PageTokenUtil.decodePageRequest(serializedPageToken, requestedPageSize); + @Nullable String serializedPageToken, + @Nullable Integer requestedPageSize, + BooleanSupplier shouldDecodeToken) { + return PageTokenUtil.decodePageRequest( + serializedPageToken, requestedPageSize, shouldDecodeToken); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageTokenUtil.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageTokenUtil.java index 8a811f41c..0b6255192 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageTokenUtil.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageTokenUtil.java @@ -40,6 +40,7 @@ import java.util.Map; import java.util.Optional; import java.util.OptionalInt; import java.util.ServiceLoader; +import java.util.function.BooleanSupplier; final class PageTokenUtil { @@ -118,8 +119,10 @@ final class PageTokenUtil { * token. */ static PageToken decodePageRequest( - @Nullable String requestedPageToken, @Nullable Integer requestedPageSize) { - if (requestedPageToken != null) { + @Nullable String requestedPageToken, + @Nullable Integer requestedPageSize, + BooleanSupplier shouldDecodeToken) { + if (requestedPageToken != null && shouldDecodeToken.getAsBoolean()) { var bytes = Base64.getUrlDecoder().decode(requestedPageToken); try { var pageToken = SMILE_MAPPER.readValue(bytes, PageToken.class); @@ -134,7 +137,7 @@ final class PageTokenUtil { } catch (IOException e) { throw new RuntimeException(e); } - } else if (requestedPageSize != null) { + } else if (requestedPageSize != null && shouldDecodeToken.getAsBoolean()) { int pageSizeInt = requestedPageSize; checkArgument(pageSizeInt >= 0, "Invalid page size"); return fromLimit(pageSizeInt); diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/pagination/PageTokenTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/pagination/PageTokenTest.java index 338bbc53f..dd5cb398c 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/persistence/pagination/PageTokenTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/persistence/pagination/PageTokenTest.java @@ -53,7 +53,7 @@ class PageTokenTest { soft.assertThat(pageEverything.encodedResponseToken()).isNull(); soft.assertThat(pageEverything.items()).containsExactly(1, 2, 3, 4); - r = PageToken.build(null, null); + r = PageToken.build(null, null, () -> true); soft.assertThat(r.paginationRequested()).isFalse(); soft.assertThat(r.pageSize()).isEmpty(); soft.assertThat(r.value()).isEmpty(); @@ -62,7 +62,7 @@ class PageTokenTest { @Test public void testLimit() { PageToken r = PageToken.fromLimit(123); - soft.assertThat(r).isEqualTo(PageToken.build(null, 123)); + soft.assertThat(r).isEqualTo(PageToken.build(null, 123, () -> true)); soft.assertThat(r.paginationRequested()).isTrue(); soft.assertThat(r.pageSize()).isEqualTo(OptionalInt.of(123)); soft.assertThat(r.value()).isEmpty(); @@ -71,7 +71,7 @@ class PageTokenTest { @Test public void testTokenValueForPaging() { PageToken r = PageToken.fromLimit(2); - soft.assertThat(r).isEqualTo(PageToken.build(null, 2)); + soft.assertThat(r).isEqualTo(PageToken.build(null, 2, () -> true)); Page<Integer> pageMoreData = Page.mapped( r, @@ -103,7 +103,7 @@ class PageTokenTest { soft.assertThat(lastPageNotSaturated.items()).containsExactly(3); r = PageToken.fromLimit(200); - soft.assertThat(r).isEqualTo(PageToken.build(null, 200)); + soft.assertThat(r).isEqualTo(PageToken.build(null, 200, () -> true)); Page<Integer> page200 = Page.mapped( r, @@ -117,8 +117,10 @@ class PageTokenTest { @ParameterizedTest @MethodSource public void testDeSer(Integer pageSize, String serializedPageToken, PageToken expectedPageToken) { - soft.assertThat(PageTokenUtil.decodePageRequest(serializedPageToken, pageSize)) + soft.assertThat(PageTokenUtil.decodePageRequest(serializedPageToken, pageSize, () -> true)) .isEqualTo(expectedPageToken); + soft.assertThat(PageTokenUtil.decodePageRequest(serializedPageToken, pageSize, () -> false)) + .isEqualTo(PageToken.readEverything()); } static Stream<Arguments> testDeSer() { @@ -142,16 +144,16 @@ class PageTokenTest { @ParameterizedTest @MethodSource public void testApiRoundTrip(Token token) { - PageToken request = PageToken.build(null, 123); + PageToken request = PageToken.build(null, 123, () -> true); Page<?> page = Page.mapped(request, Stream.of("i1"), Function.identity(), x -> token); soft.assertThat(page.encodedResponseToken()).isNotBlank(); - PageToken r = PageToken.build(page.encodedResponseToken(), null); + PageToken r = PageToken.build(page.encodedResponseToken(), null, () -> true); soft.assertThat(r.value()).contains(token); soft.assertThat(r.paginationRequested()).isTrue(); soft.assertThat(r.pageSize()).isEqualTo(OptionalInt.of(123)); - r = PageToken.build(page.encodedResponseToken(), 456); + r = PageToken.build(page.encodedResponseToken(), 456, () -> true); soft.assertThat(r.value()).contains(token); soft.assertThat(r.paginationRequested()).isTrue(); soft.assertThat(r.pageSize()).isEqualTo(OptionalInt.of(456)); diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index b0cfc01b1..17cdd7af3 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -18,6 +18,8 @@ */ package org.apache.polaris.service.catalog.iceberg; +import static org.apache.polaris.core.config.FeatureConfiguration.LIST_PAGINATION_ENABLED; + import com.google.common.base.Preconditions; import com.google.common.collect.Maps; import io.smallrye.common.annotation.Identifier; @@ -187,13 +189,17 @@ public class IcebergCatalogHandler extends CatalogHandler implements AutoCloseab return isCreate; } + private boolean shouldDecodeToken() { + return realmConfig.getConfig(LIST_PAGINATION_ENABLED, getResolvedCatalogEntity()); + } + public ListNamespacesResponse listNamespaces( Namespace parent, String pageToken, Integer pageSize) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES; authorizeBasicNamespaceOperationOrThrow(op, parent); if (baseCatalog instanceof IcebergCatalog polarisCatalog) { - PageToken pageRequest = PageToken.build(pageToken, pageSize); + PageToken pageRequest = PageToken.build(pageToken, pageSize, this::shouldDecodeToken); Page<Namespace> results = polarisCatalog.listNamespaces(parent, pageRequest); return ListNamespacesResponse.builder() .addAll(results.items()) @@ -332,7 +338,7 @@ public class IcebergCatalogHandler extends CatalogHandler implements AutoCloseab authorizeBasicNamespaceOperationOrThrow(op, namespace); if (baseCatalog instanceof IcebergCatalog polarisCatalog) { - PageToken pageRequest = PageToken.build(pageToken, pageSize); + PageToken pageRequest = PageToken.build(pageToken, pageSize, this::shouldDecodeToken); Page<TableIdentifier> results = polarisCatalog.listTables(namespace, pageRequest); return ListTablesResponse.builder() .addAll(results.items()) @@ -935,7 +941,7 @@ public class IcebergCatalogHandler extends CatalogHandler implements AutoCloseab authorizeBasicNamespaceOperationOrThrow(op, namespace); if (baseCatalog instanceof IcebergCatalog polarisCatalog) { - PageToken pageRequest = PageToken.build(pageToken, pageSize); + PageToken pageRequest = PageToken.build(pageToken, pageSize, this::shouldDecodeToken); Page<TableIdentifier> results = polarisCatalog.listViews(namespace, pageRequest); return ListTablesResponse.builder() .addAll(results.items()) diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java index 38c4db7e5..d10f82057 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java @@ -2240,7 +2240,7 @@ public abstract class AbstractIcebergCatalogTest extends CatalogTests<IcebergCat } private static PageToken nextRequest(Page<?> previousPage) { - return PageToken.build(previousPage.encodedResponseToken(), null); + return PageToken.build(previousPage.encodedResponseToken(), null, () -> true); } @Test