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 <[email protected]>
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
