Skip to content

Commit a0a2b87

Browse files
authored
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.
1 parent c97b150 commit a0a2b87

File tree

8 files changed

+97
-17
lines changed

8 files changed

+97
-17
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ the authentication parameters are picked from the environment or configuration f
4747
- The `DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED` feature was added to support placing tables
4848
at locations that better optimize for object storage.
4949

50+
- The `LIST_PAGINATION_ENABLED` (default: false) feature flag can be used to enable pagination
51+
in the Iceberg REST Catalog API.
52+
5053
- The Helm chart now supports Pod Disruption Budgets (PDBs) for Polaris components. This allows users to define
5154
the minimum number of pods that must be available during voluntary disruptions, such as node maintenance.
5255

integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,20 @@ public List<TableIdentifier> listViews(String catalog, Namespace namespace) {
205205
}
206206
}
207207

208+
public ListTablesResponse listViews(
209+
String catalog, Namespace namespace, String pageToken, String pageSize) {
210+
String ns = RESTUtil.encodeNamespace(namespace);
211+
Map<String, String> queryParams = new HashMap<>();
212+
queryParams.put("pageToken", pageToken);
213+
queryParams.put("pageSize", pageSize);
214+
try (Response res =
215+
request("v1/{cat}/namespaces/" + ns + "/views", Map.of("cat", catalog), queryParams)
216+
.get()) {
217+
assertThat(res.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
218+
return res.readEntity(ListTablesResponse.class);
219+
}
220+
}
221+
208222
public void dropView(String catalog, TableIdentifier id) {
209223
String ns = RESTUtil.encodeNamespace(id.namespace());
210224
try (Response res =

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2160,4 +2160,52 @@ public void testPaginatedListTables() {
21602160
restCatalog.dropNamespace(namespace);
21612161
}
21622162
}
2163+
2164+
@Test
2165+
public void testNonPaginatedListTablesViewNamespaces() {
2166+
Catalog catalog = managementApi.getCatalog(currentCatalogName);
2167+
Map<String, String> catalogProps = new HashMap<>(catalog.getProperties().toMap());
2168+
catalogProps.put(FeatureConfiguration.LIST_PAGINATION_ENABLED.catalogConfig(), "false");
2169+
managementApi.updateCatalog(catalog, catalogProps);
2170+
2171+
String prefix = "testNonPaginatedListTablesViewNamespaces";
2172+
Namespace namespace = Namespace.of(prefix);
2173+
restCatalog.createNamespace(namespace);
2174+
for (int i = 0; i < 5; i++) {
2175+
restCatalog.createNamespace(Namespace.of(prefix, "nested-ns" + i));
2176+
restCatalog.createTable(TableIdentifier.of(namespace, "table" + i), SCHEMA);
2177+
restCatalog
2178+
.buildView(TableIdentifier.of(namespace, "view" + i))
2179+
.withSchema(SCHEMA)
2180+
.withDefaultNamespace(namespace)
2181+
.withQuery("spark", VIEW_QUERY)
2182+
.create();
2183+
}
2184+
2185+
assertThat(catalogApi.listTables(currentCatalogName, namespace)).hasSize(5);
2186+
// Note: no pagination per feature config
2187+
ListTablesResponse response = catalogApi.listTables(currentCatalogName, namespace, null, "2");
2188+
assertThat(response.identifiers()).hasSize(5);
2189+
assertThat(response.nextPageToken()).isNull();
2190+
response = catalogApi.listTables(currentCatalogName, namespace, "fake-token", null);
2191+
assertThat(response.identifiers()).hasSize(5);
2192+
assertThat(response.nextPageToken()).isNull();
2193+
2194+
assertThat(catalogApi.listViews(currentCatalogName, namespace)).hasSize(5);
2195+
response = catalogApi.listViews(currentCatalogName, namespace, null, "2");
2196+
assertThat(response.identifiers()).hasSize(5);
2197+
assertThat(response.nextPageToken()).isNull();
2198+
response = catalogApi.listViews(currentCatalogName, namespace, "fake-token", null);
2199+
assertThat(response.identifiers()).hasSize(5);
2200+
assertThat(response.nextPageToken()).isNull();
2201+
2202+
assertThat(catalogApi.listNamespaces(currentCatalogName, namespace)).hasSize(5);
2203+
ListNamespacesResponse nsResponse =
2204+
catalogApi.listNamespaces(currentCatalogName, namespace, null, "2");
2205+
assertThat(nsResponse.namespaces()).hasSize(5);
2206+
assertThat(nsResponse.nextPageToken()).isNull();
2207+
nsResponse = catalogApi.listNamespaces(currentCatalogName, namespace, "fake-token", null);
2208+
assertThat(nsResponse.namespaces()).hasSize(5);
2209+
assertThat(nsResponse.nextPageToken()).isNull();
2210+
}
21632211
}

polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageToken.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import jakarta.annotation.Nullable;
2525
import java.util.Optional;
2626
import java.util.OptionalInt;
27+
import java.util.function.BooleanSupplier;
2728
import org.apache.polaris.immutables.PolarisImmutable;
2829

2930
/** A wrapper for pagination information passed in as part of a request. */
@@ -86,7 +87,10 @@ static PageToken fromLimit(int limit) {
8687
* @see Page#encodedResponseToken()
8788
*/
8889
static PageToken build(
89-
@Nullable String serializedPageToken, @Nullable Integer requestedPageSize) {
90-
return PageTokenUtil.decodePageRequest(serializedPageToken, requestedPageSize);
90+
@Nullable String serializedPageToken,
91+
@Nullable Integer requestedPageSize,
92+
BooleanSupplier shouldDecodeToken) {
93+
return PageTokenUtil.decodePageRequest(
94+
serializedPageToken, requestedPageSize, shouldDecodeToken);
9195
}
9296
}

polaris-core/src/main/java/org/apache/polaris/core/persistence/pagination/PageTokenUtil.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.util.Optional;
4141
import java.util.OptionalInt;
4242
import java.util.ServiceLoader;
43+
import java.util.function.BooleanSupplier;
4344

4445
final class PageTokenUtil {
4546

@@ -118,8 +119,10 @@ private PageTokenUtil() {}
118119
* token.
119120
*/
120121
static PageToken decodePageRequest(
121-
@Nullable String requestedPageToken, @Nullable Integer requestedPageSize) {
122-
if (requestedPageToken != null) {
122+
@Nullable String requestedPageToken,
123+
@Nullable Integer requestedPageSize,
124+
BooleanSupplier shouldDecodeToken) {
125+
if (requestedPageToken != null && shouldDecodeToken.getAsBoolean()) {
123126
var bytes = Base64.getUrlDecoder().decode(requestedPageToken);
124127
try {
125128
var pageToken = SMILE_MAPPER.readValue(bytes, PageToken.class);
@@ -134,7 +137,7 @@ static PageToken decodePageRequest(
134137
} catch (IOException e) {
135138
throw new RuntimeException(e);
136139
}
137-
} else if (requestedPageSize != null) {
140+
} else if (requestedPageSize != null && shouldDecodeToken.getAsBoolean()) {
138141
int pageSizeInt = requestedPageSize;
139142
checkArgument(pageSizeInt >= 0, "Invalid page size");
140143
return fromLimit(pageSizeInt);

polaris-core/src/test/java/org/apache/polaris/core/persistence/pagination/PageTokenTest.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public void testReadEverything() {
5353
soft.assertThat(pageEverything.encodedResponseToken()).isNull();
5454
soft.assertThat(pageEverything.items()).containsExactly(1, 2, 3, 4);
5555

56-
r = PageToken.build(null, null);
56+
r = PageToken.build(null, null, () -> true);
5757
soft.assertThat(r.paginationRequested()).isFalse();
5858
soft.assertThat(r.pageSize()).isEmpty();
5959
soft.assertThat(r.value()).isEmpty();
@@ -62,7 +62,7 @@ public void testReadEverything() {
6262
@Test
6363
public void testLimit() {
6464
PageToken r = PageToken.fromLimit(123);
65-
soft.assertThat(r).isEqualTo(PageToken.build(null, 123));
65+
soft.assertThat(r).isEqualTo(PageToken.build(null, 123, () -> true));
6666
soft.assertThat(r.paginationRequested()).isTrue();
6767
soft.assertThat(r.pageSize()).isEqualTo(OptionalInt.of(123));
6868
soft.assertThat(r.value()).isEmpty();
@@ -71,7 +71,7 @@ public void testLimit() {
7171
@Test
7272
public void testTokenValueForPaging() {
7373
PageToken r = PageToken.fromLimit(2);
74-
soft.assertThat(r).isEqualTo(PageToken.build(null, 2));
74+
soft.assertThat(r).isEqualTo(PageToken.build(null, 2, () -> true));
7575
Page<Integer> pageMoreData =
7676
Page.mapped(
7777
r,
@@ -103,7 +103,7 @@ public void testTokenValueForPaging() {
103103
soft.assertThat(lastPageNotSaturated.items()).containsExactly(3);
104104

105105
r = PageToken.fromLimit(200);
106-
soft.assertThat(r).isEqualTo(PageToken.build(null, 200));
106+
soft.assertThat(r).isEqualTo(PageToken.build(null, 200, () -> true));
107107
Page<Integer> page200 =
108108
Page.mapped(
109109
r,
@@ -117,8 +117,10 @@ public void testTokenValueForPaging() {
117117
@ParameterizedTest
118118
@MethodSource
119119
public void testDeSer(Integer pageSize, String serializedPageToken, PageToken expectedPageToken) {
120-
soft.assertThat(PageTokenUtil.decodePageRequest(serializedPageToken, pageSize))
120+
soft.assertThat(PageTokenUtil.decodePageRequest(serializedPageToken, pageSize, () -> true))
121121
.isEqualTo(expectedPageToken);
122+
soft.assertThat(PageTokenUtil.decodePageRequest(serializedPageToken, pageSize, () -> false))
123+
.isEqualTo(PageToken.readEverything());
122124
}
123125

124126
static Stream<Arguments> testDeSer() {
@@ -142,16 +144,16 @@ static Stream<Arguments> testDeSer() {
142144
@ParameterizedTest
143145
@MethodSource
144146
public void testApiRoundTrip(Token token) {
145-
PageToken request = PageToken.build(null, 123);
147+
PageToken request = PageToken.build(null, 123, () -> true);
146148
Page<?> page = Page.mapped(request, Stream.of("i1"), Function.identity(), x -> token);
147149
soft.assertThat(page.encodedResponseToken()).isNotBlank();
148150

149-
PageToken r = PageToken.build(page.encodedResponseToken(), null);
151+
PageToken r = PageToken.build(page.encodedResponseToken(), null, () -> true);
150152
soft.assertThat(r.value()).contains(token);
151153
soft.assertThat(r.paginationRequested()).isTrue();
152154
soft.assertThat(r.pageSize()).isEqualTo(OptionalInt.of(123));
153155

154-
r = PageToken.build(page.encodedResponseToken(), 456);
156+
r = PageToken.build(page.encodedResponseToken(), 456, () -> true);
155157
soft.assertThat(r.value()).contains(token);
156158
soft.assertThat(r.paginationRequested()).isTrue();
157159
soft.assertThat(r.pageSize()).isEqualTo(OptionalInt.of(456));

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
*/
1919
package org.apache.polaris.service.catalog.iceberg;
2020

21+
import static org.apache.polaris.core.config.FeatureConfiguration.LIST_PAGINATION_ENABLED;
22+
2123
import com.google.common.base.Preconditions;
2224
import com.google.common.collect.Maps;
2325
import io.smallrye.common.annotation.Identifier;
@@ -187,13 +189,17 @@ public static boolean isCreate(UpdateTableRequest request) {
187189
return isCreate;
188190
}
189191

192+
private boolean shouldDecodeToken() {
193+
return realmConfig.getConfig(LIST_PAGINATION_ENABLED, getResolvedCatalogEntity());
194+
}
195+
190196
public ListNamespacesResponse listNamespaces(
191197
Namespace parent, String pageToken, Integer pageSize) {
192198
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
193199
authorizeBasicNamespaceOperationOrThrow(op, parent);
194200

195201
if (baseCatalog instanceof IcebergCatalog polarisCatalog) {
196-
PageToken pageRequest = PageToken.build(pageToken, pageSize);
202+
PageToken pageRequest = PageToken.build(pageToken, pageSize, this::shouldDecodeToken);
197203
Page<Namespace> results = polarisCatalog.listNamespaces(parent, pageRequest);
198204
return ListNamespacesResponse.builder()
199205
.addAll(results.items())
@@ -332,7 +338,7 @@ public ListTablesResponse listTables(Namespace namespace, String pageToken, Inte
332338
authorizeBasicNamespaceOperationOrThrow(op, namespace);
333339

334340
if (baseCatalog instanceof IcebergCatalog polarisCatalog) {
335-
PageToken pageRequest = PageToken.build(pageToken, pageSize);
341+
PageToken pageRequest = PageToken.build(pageToken, pageSize, this::shouldDecodeToken);
336342
Page<TableIdentifier> results = polarisCatalog.listTables(namespace, pageRequest);
337343
return ListTablesResponse.builder()
338344
.addAll(results.items())
@@ -935,7 +941,7 @@ public ListTablesResponse listViews(Namespace namespace, String pageToken, Integ
935941
authorizeBasicNamespaceOperationOrThrow(op, namespace);
936942

937943
if (baseCatalog instanceof IcebergCatalog polarisCatalog) {
938-
PageToken pageRequest = PageToken.build(pageToken, pageSize);
944+
PageToken pageRequest = PageToken.build(pageToken, pageSize, this::shouldDecodeToken);
939945
Page<TableIdentifier> results = polarisCatalog.listViews(namespace, pageRequest);
940946
return ListTablesResponse.builder()
941947
.addAll(results.items())

runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2240,7 +2240,7 @@ public void testEventsAreEmitted() {
22402240
}
22412241

22422242
private static PageToken nextRequest(Page<?> previousPage) {
2243-
return PageToken.build(previousPage.encodedResponseToken(), null);
2243+
return PageToken.build(previousPage.encodedResponseToken(), null, () -> true);
22442244
}
22452245

22462246
@Test

0 commit comments

Comments
 (0)