Skip to content

Commit 276e418

Browse files
eric-maynarderic-maynard
authored
Revert "Extensible pagination token implementation (#1938)"
This reverts commit fb418a2.
1 parent 987c554 commit 276e418

File tree

32 files changed

+506
-1329
lines changed

32 files changed

+506
-1329
lines changed

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

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -101,24 +101,6 @@ public List<Namespace> listNamespaces(String catalog, Namespace parent) {
101101
}
102102
}
103103

104-
public ListNamespacesResponse listNamespaces(
105-
String catalog, Namespace parent, String pageToken, String pageSize) {
106-
Map<String, String> queryParams = new HashMap<>();
107-
if (!parent.isEmpty()) {
108-
// TODO change this for Iceberg 1.7.2:
109-
// queryParams.put("parent", RESTUtil.encodeNamespace(parent));
110-
queryParams.put("parent", Joiner.on('\u001f').join(parent.levels()));
111-
}
112-
queryParams.put("pageToken", pageToken);
113-
queryParams.put("pageSize", pageSize);
114-
try (Response response =
115-
request("v1/{cat}/namespaces", Map.of("cat", catalog), queryParams).get()) {
116-
assertThat(response.getStatus()).isEqualTo(OK.getStatusCode());
117-
ListNamespacesResponse res = response.readEntity(ListNamespacesResponse.class);
118-
return res;
119-
}
120-
}
121-
122104
public List<Namespace> listAllNamespacesChildFirst(String catalog) {
123105
List<Namespace> result = new ArrayList<>();
124106
for (int idx = -1; idx < result.size(); idx++) {
@@ -160,20 +142,6 @@ public List<TableIdentifier> listTables(String catalog, Namespace namespace) {
160142
}
161143
}
162144

163-
public ListTablesResponse listTables(
164-
String catalog, Namespace namespace, String pageToken, String pageSize) {
165-
String ns = RESTUtil.encodeNamespace(namespace);
166-
Map<String, String> queryParams = new HashMap<>();
167-
queryParams.put("pageToken", pageToken);
168-
queryParams.put("pageSize", pageSize);
169-
try (Response res =
170-
request("v1/{cat}/namespaces/" + ns + "/tables", Map.of("cat", catalog), queryParams)
171-
.get()) {
172-
assertThat(res.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
173-
return res.readEntity(ListTablesResponse.class);
174-
}
175-
}
176-
177145
public void dropTable(String catalog, TableIdentifier id) {
178146
String ns = RESTUtil.encodeNamespace(id.namespace());
179147
try (Response res =

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

Lines changed: 1 addition & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,6 @@
6969
import org.apache.iceberg.rest.RESTUtil;
7070
import org.apache.iceberg.rest.requests.CreateTableRequest;
7171
import org.apache.iceberg.rest.responses.ErrorResponse;
72-
import org.apache.iceberg.rest.responses.ListNamespacesResponse;
73-
import org.apache.iceberg.rest.responses.ListTablesResponse;
7472
import org.apache.iceberg.rest.responses.LoadTableResponse;
7573
import org.apache.iceberg.types.Types;
7674
import org.apache.polaris.core.admin.model.Catalog;
@@ -163,8 +161,7 @@ public abstract class PolarisRestCatalogIntegrationBase extends CatalogTests<RES
163161

164162
private static final String[] DEFAULT_CATALOG_PROPERTIES = {
165163
"polaris.config.allow.unstructured.table.location", "true",
166-
"polaris.config.allow.external.table.location", "true",
167-
"polaris.config.list-pagination-enabled", "true"
164+
"polaris.config.allow.external.table.location", "true"
168165
};
169166

170167
@Retention(RetentionPolicy.RUNTIME)
@@ -2026,72 +2023,4 @@ public void testETagChangeAfterDMLOperations() {
20262023
assertThat(currentETag).isEqualTo(afterDMLETag); // Should match post-DML ETag
20272024
}
20282025
}
2029-
2030-
@Test
2031-
public void testPaginatedListNamespaces() {
2032-
String prefix = "testPaginatedListNamespaces";
2033-
for (int i = 0; i < 20; i++) {
2034-
Namespace namespace = Namespace.of(prefix + i);
2035-
restCatalog.createNamespace(namespace);
2036-
}
2037-
2038-
try {
2039-
Assertions.assertThat(catalogApi.listNamespaces(currentCatalogName, Namespace.empty()))
2040-
.hasSize(20);
2041-
for (var pageSize : List.of(1, 2, 3, 9, 10, 11, 19, 20, 21, 2000)) {
2042-
int total = 0;
2043-
String pageToken = null;
2044-
do {
2045-
ListNamespacesResponse response =
2046-
catalogApi.listNamespaces(
2047-
currentCatalogName, Namespace.empty(), pageToken, String.valueOf(pageSize));
2048-
Assertions.assertThat(response.namespaces().size()).isLessThanOrEqualTo(pageSize);
2049-
total += response.namespaces().size();
2050-
pageToken = response.nextPageToken();
2051-
} while (pageToken != null);
2052-
Assertions.assertThat(total)
2053-
.as("Total paginated results for pageSize = " + pageSize)
2054-
.isEqualTo(20);
2055-
}
2056-
} finally {
2057-
for (int i = 0; i < 20; i++) {
2058-
Namespace namespace = Namespace.of(prefix + i);
2059-
restCatalog.dropNamespace(namespace);
2060-
}
2061-
}
2062-
}
2063-
2064-
@Test
2065-
public void testPaginatedListTables() {
2066-
String prefix = "testPaginatedListTables";
2067-
Namespace namespace = Namespace.of(prefix);
2068-
restCatalog.createNamespace(namespace);
2069-
for (int i = 0; i < 20; i++) {
2070-
restCatalog.createTable(TableIdentifier.of(namespace, prefix + i), SCHEMA);
2071-
}
2072-
2073-
try {
2074-
Assertions.assertThat(catalogApi.listTables(currentCatalogName, namespace)).hasSize(20);
2075-
for (var pageSize : List.of(1, 2, 3, 9, 10, 11, 19, 20, 21, 2000)) {
2076-
int total = 0;
2077-
String pageToken = null;
2078-
do {
2079-
ListTablesResponse response =
2080-
catalogApi.listTables(
2081-
currentCatalogName, namespace, pageToken, String.valueOf(pageSize));
2082-
Assertions.assertThat(response.identifiers().size()).isLessThanOrEqualTo(pageSize);
2083-
total += response.identifiers().size();
2084-
pageToken = response.nextPageToken();
2085-
} while (pageToken != null);
2086-
Assertions.assertThat(total)
2087-
.as("Total paginated results for pageSize = " + pageSize)
2088-
.isEqualTo(20);
2089-
}
2090-
} finally {
2091-
for (int i = 0; i < 20; i++) {
2092-
restCatalog.dropTable(TableIdentifier.of(namespace, prefix + i));
2093-
}
2094-
restCatalog.dropNamespace(namespace);
2095-
}
2096-
}
20972026
}

‎persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
import org.apache.polaris.core.persistence.BaseMetaStoreManager;
5757
import org.apache.polaris.core.persistence.PrincipalSecretsGenerator;
5858
import org.apache.polaris.core.persistence.RetryOnConcurrencyException;
59-
import org.apache.polaris.core.persistence.pagination.EntityIdToken;
59+
import org.apache.polaris.core.persistence.pagination.HasPageSize;
6060
import org.apache.polaris.core.persistence.pagination.Page;
6161
import org.apache.polaris.core.persistence.pagination.PageToken;
6262
import org.apache.polaris.core.persistence.transactional.AbstractTransactionalPersistence;
@@ -480,7 +480,11 @@ public List<EntityNameLookupRecord> lookupEntityActiveBatchInCurrentTxn(
480480
.map(ModelEntity::toEntity)
481481
.filter(entityFilter);
482482

483-
return Page.mapped(pageToken, data, transformer, EntityIdToken::fromEntity);
483+
if (pageToken instanceof HasPageSize hasPageSize) {
484+
data = data.limit(hasPageSize.getPageSize());
485+
}
486+
487+
return Page.fromItems(data.map(transformer).collect(Collectors.toList()));
484488
}
485489

486490
/** {@inheritDoc} */

‎persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import org.apache.polaris.core.entity.PolarisEntityType;
3636
import org.apache.polaris.core.entity.PolarisGrantRecord;
3737
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
38-
import org.apache.polaris.core.persistence.pagination.EntityIdToken;
3938
import org.apache.polaris.core.persistence.pagination.PageToken;
4039
import org.apache.polaris.core.policy.PolarisPolicyMappingRecord;
4140
import org.apache.polaris.core.policy.PolicyEntity;
@@ -295,17 +294,7 @@ List<ModelEntity> lookupFullEntitiesActive(
295294

296295
// Currently check against ENTITIES not joining with ENTITIES_ACTIVE
297296
String hql =
298-
"SELECT m from ModelEntity m where"
299-
+ " m.catalogId=:catalogId and m.parentId=:parentId and m.typeCode=:typeCode";
300-
301-
var entityIdToken = pageToken.valueAs(EntityIdToken.class);
302-
if (entityIdToken.isPresent()) {
303-
hql += " and m.id > :tokenId";
304-
}
305-
306-
if (pageToken.paginationRequested()) {
307-
hql += " order by m.id asc";
308-
}
297+
"SELECT m from ModelEntity m where m.catalogId=:catalogId and m.parentId=:parentId and m.typeCode=:typeCode";
309298

310299
TypedQuery<ModelEntity> query =
311300
session
@@ -314,11 +303,6 @@ List<ModelEntity> lookupFullEntitiesActive(
314303
.setParameter("parentId", parentId)
315304
.setParameter("typeCode", entityType.getCode());
316305

317-
if (entityIdToken.isPresent()) {
318-
long tokenId = entityIdToken.get().entityId();
319-
query = query.setParameter("tokenId", tokenId);
320-
}
321-
322306
return query.getResultList();
323307
}
324308

‎persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import java.util.List;
3333
import java.util.Map;
3434
import java.util.Optional;
35-
import java.util.concurrent.atomic.AtomicReference;
3635
import java.util.function.Function;
3736
import java.util.function.Predicate;
3837
import java.util.stream.Collectors;
@@ -54,7 +53,7 @@
5453
import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException;
5554
import org.apache.polaris.core.persistence.PrincipalSecretsGenerator;
5655
import org.apache.polaris.core.persistence.RetryOnConcurrencyException;
57-
import org.apache.polaris.core.persistence.pagination.EntityIdToken;
56+
import org.apache.polaris.core.persistence.pagination.HasPageSize;
5857
import org.apache.polaris.core.persistence.pagination.Page;
5958
import org.apache.polaris.core.persistence.pagination.PageToken;
6059
import org.apache.polaris.core.policy.PolarisPolicyMappingRecord;
@@ -460,7 +459,7 @@ public <T> Page<T> listEntities(
460459
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
461460
@Nonnull Function<PolarisBaseEntity, T> transformer,
462461
@Nonnull PageToken pageToken) {
463-
Map<String, Object> whereEquals =
462+
Map<String, Object> params =
464463
Map.of(
465464
"catalog_id",
466465
catalogId,
@@ -470,41 +469,29 @@ public <T> Page<T> listEntities(
470469
entityType.getCode(),
471470
"realm_id",
472471
realmId);
473-
Map<String, Object> whereGreater;
474472

475473
// Limit can't be pushed down, due to client side filtering
476474
// absence of transaction.
477-
String orderByColumnName = null;
478-
if (pageToken.paginationRequested()) {
479-
orderByColumnName = ModelEntity.ID_COLUMN;
480-
whereGreater =
481-
pageToken
482-
.valueAs(EntityIdToken.class)
483-
.map(
484-
entityIdToken ->
485-
Map.<String, Object>of(ModelEntity.ID_COLUMN, entityIdToken.entityId()))
486-
.orElse(Map.of());
487-
} else {
488-
whereGreater = Map.of();
489-
}
490-
491475
try {
492476
PreparedQuery query =
493477
QueryGenerator.generateSelectQuery(
494-
ModelEntity.ALL_COLUMNS,
495-
ModelEntity.TABLE_NAME,
496-
whereEquals,
497-
whereGreater,
498-
orderByColumnName);
499-
AtomicReference<Page<T>> results = new AtomicReference<>();
478+
ModelEntity.ALL_COLUMNS, ModelEntity.TABLE_NAME, params);
479+
List<PolarisBaseEntity> results = new ArrayList<>();
500480
datasourceOperations.executeSelectOverStream(
501481
query,
502482
new ModelEntity(),
503483
stream -> {
504484
var data = stream.filter(entityFilter);
505-
results.set(Page.mapped(pageToken, data, transformer, EntityIdToken::fromEntity));
485+
if (pageToken instanceof HasPageSize hasPageSize) {
486+
data = data.limit(hasPageSize.getPageSize());
487+
}
488+
data.forEach(results::add);
506489
});
507-
return results.get();
490+
List<T> resultsOrEmpty =
491+
results == null
492+
? Collections.emptyList()
493+
: results.stream().filter(entityFilter).map(transformer).collect(Collectors.toList());
494+
return Page.fromItems(resultsOrEmpty);
508495
} catch (SQLException e) {
509496
throw new RuntimeException(
510497
String.format("Failed to retrieve polaris entities due to %s", e.getMessage()), e);

‎persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java

Lines changed: 9 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import com.google.common.annotations.VisibleForTesting;
2222
import jakarta.annotation.Nonnull;
23-
import jakarta.annotation.Nullable;
2423
import java.util.ArrayList;
2524
import java.util.Arrays;
2625
import java.util.Collections;
@@ -60,27 +59,8 @@ public static PreparedQuery generateSelectQuery(
6059
@Nonnull List<String> projections,
6160
@Nonnull String tableName,
6261
@Nonnull Map<String, Object> whereClause) {
63-
return generateSelectQuery(projections, tableName, whereClause, Map.of(), null);
64-
}
65-
66-
/**
67-
* Generates a SELECT query with projection and filtering.
68-
*
69-
* @param projections List of columns to retrieve.
70-
* @param tableName Target table name.
71-
* @param whereEquals Column-value pairs used in WHERE filtering.
72-
* @return A parameterized SELECT query.
73-
* @throws IllegalArgumentException if any whereClause column isn't in projections.
74-
*/
75-
public static PreparedQuery generateSelectQuery(
76-
@Nonnull List<String> projections,
77-
@Nonnull String tableName,
78-
@Nonnull Map<String, Object> whereEquals,
79-
@Nonnull Map<String, Object> whereGreater,
80-
@Nullable String orderByColumn) {
81-
QueryFragment where =
82-
generateWhereClause(new HashSet<>(projections), whereEquals, whereGreater);
83-
PreparedQuery query = generateSelectQuery(projections, tableName, where.sql(), orderByColumn);
62+
QueryFragment where = generateWhereClause(new HashSet<>(projections), whereClause);
63+
PreparedQuery query = generateSelectQuery(projections, tableName, where.sql());
8464
return new PreparedQuery(query.sql(), where.parameters());
8565
}
8666

@@ -128,8 +108,7 @@ public static PreparedQuery generateSelectQueryWithEntityIds(
128108
params.add(realmId);
129109
String where = " WHERE (catalog_id, id) IN (" + placeholders + ") AND realm_id = ?";
130110
return new PreparedQuery(
131-
generateSelectQuery(ModelEntity.ALL_COLUMNS, ModelEntity.TABLE_NAME, where, null).sql(),
132-
params);
111+
generateSelectQuery(ModelEntity.ALL_COLUMNS, ModelEntity.TABLE_NAME, where).sql(), params);
133112
}
134113

135114
/**
@@ -178,7 +157,7 @@ public static PreparedQuery generateUpdateQuery(
178157
@Nonnull List<Object> values,
179158
@Nonnull Map<String, Object> whereClause) {
180159
List<Object> bindingParams = new ArrayList<>(values);
181-
QueryFragment where = generateWhereClause(new HashSet<>(allColumns), whereClause, Map.of());
160+
QueryFragment where = generateWhereClause(new HashSet<>(allColumns), whereClause);
182161
String setClause = allColumns.stream().map(c -> c + " = ?").collect(Collectors.joining(", "));
183162
String sql =
184163
"UPDATE " + getFullyQualifiedTableName(tableName) + " SET " + setClause + where.sql();
@@ -198,49 +177,34 @@ public static PreparedQuery generateDeleteQuery(
198177
@Nonnull List<String> tableColumns,
199178
@Nonnull String tableName,
200179
@Nonnull Map<String, Object> whereClause) {
201-
QueryFragment where = generateWhereClause(new HashSet<>(tableColumns), whereClause, Map.of());
180+
QueryFragment where = generateWhereClause(new HashSet<>(tableColumns), whereClause);
202181
return new PreparedQuery(
203182
"DELETE FROM " + getFullyQualifiedTableName(tableName) + where.sql(), where.parameters());
204183
}
205184

206185
private static PreparedQuery generateSelectQuery(
207-
@Nonnull List<String> columnNames,
208-
@Nonnull String tableName,
209-
@Nonnull String filter,
210-
@Nullable String orderByColumn) {
186+
@Nonnull List<String> columnNames, @Nonnull String tableName, @Nonnull String filter) {
211187
String sql =
212188
"SELECT "
213189
+ String.join(", ", columnNames)
214190
+ " FROM "
215191
+ getFullyQualifiedTableName(tableName)
216192
+ filter;
217-
if (orderByColumn != null) {
218-
sql += " ORDER BY " + orderByColumn + " ASC";
219-
}
220193
return new PreparedQuery(sql, Collections.emptyList());
221194
}
222195

223196
@VisibleForTesting
224197
static QueryFragment generateWhereClause(
225-
@Nonnull Set<String> tableColumns,
226-
@Nonnull Map<String, Object> whereEquals,
227-
@Nonnull Map<String, Object> whereGreater) {
198+
@Nonnull Set<String> tableColumns, @Nonnull Map<String, Object> whereClause) {
228199
List<String> conditions = new ArrayList<>();
229200
List<Object> parameters = new ArrayList<>();
230-
for (Map.Entry<String, Object> entry : whereEquals.entrySet()) {
201+
for (Map.Entry<String, Object> entry : whereClause.entrySet()) {
231202
if (!tableColumns.contains(entry.getKey()) && !entry.getKey().equals("realm_id")) {
232203
throw new IllegalArgumentException("Invalid query column: " + entry.getKey());
233204
}
234205
conditions.add(entry.getKey() + " = ?");
235206
parameters.add(entry.getValue());
236207
}
237-
for (Map.Entry<String, Object> entry : whereGreater.entrySet()) {
238-
if (!tableColumns.contains(entry.getKey()) && !entry.getKey().equals("realm_id")) {
239-
throw new IllegalArgumentException("Invalid query column: " + entry.getKey());
240-
}
241-
conditions.add(entry.getKey() + " > ?");
242-
parameters.add(entry.getValue());
243-
}
244208
String clause = conditions.isEmpty() ? "" : " WHERE " + String.join(" AND ", conditions);
245209
return new QueryFragment(clause, parameters);
246210
}
@@ -294,7 +258,7 @@ public static PreparedQuery generateOverlapQuery(
294258

295259
QueryFragment where = new QueryFragment(clause, finalParams);
296260
PreparedQuery query =
297-
generateSelectQuery(ModelEntity.ALL_COLUMNS, ModelEntity.TABLE_NAME, where.sql(), null);
261+
generateSelectQuery(ModelEntity.ALL_COLUMNS, ModelEntity.TABLE_NAME, where.sql());
298262
return new PreparedQuery(query.sql(), where.parameters());
299263
}
300264

0 commit comments

Comments
 (0)