Skip to content

Commit

Permalink
Core: Update REST CatalogHandlers to handle page sizes exceeding numb…
Browse files Browse the repository at this point in the history 
…er of Namespaces/Tables/Views (#11143)
  • Loading branch information
@rcjverhoef
rcjverhoef authored Sep 30, 2024
1 parent 9454927 commit 97c9c53
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 41 deletions.
58 changes: 27 additions & 31 deletions core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -71,6 +72,7 @@
import org.apache.iceberg.rest.responses.LoadTableResponse;
import org.apache.iceberg.rest.responses.LoadViewResponse;
import org.apache.iceberg.rest.responses.UpdateNamespacePropertiesResponse;
import org.apache.iceberg.util.Pair;
import org.apache.iceberg.util.Tasks;
import org.apache.iceberg.view.BaseView;
import org.apache.iceberg.view.SQLViewRepresentation;
Expand All @@ -82,7 +84,7 @@

public class CatalogHandlers {
private static final Schema EMPTY_SCHEMA = new Schema();
private static final String INTIAL_PAGE_TOKEN = "";
private static final String INITIAL_PAGE_TOKEN = "";

private CatalogHandlers() {}

Expand All @@ -108,6 +110,19 @@ public CommitFailedException wrapped() {
}
}

private static <T> Pair<List<T>, String> paginate(List<T> list, String pageToken, int pageSize) {
int pageStart = INITIAL_PAGE_TOKEN.equals(pageToken) ? 0 : Integer.parseInt(pageToken);
if (pageStart >= list.size()) {
return Pair.of(Collections.emptyList(), null);
}

int end = Math.min(pageStart + pageSize, list.size());
List<T> subList = list.subList(pageStart, end);
String nextPageToken = end >= list.size() ? null : String.valueOf(end);

return Pair.of(subList, nextPageToken);
}

public static ListNamespacesResponse listNamespaces(
SupportsNamespaces catalog, Namespace parent) {
List<Namespace> results;
Expand All @@ -123,24 +138,19 @@ public static ListNamespacesResponse listNamespaces(
public static ListNamespacesResponse listNamespaces(
SupportsNamespaces catalog, Namespace parent, String pageToken, String pageSize) {
List<Namespace> results;
List<Namespace> subResults;

if (parent.isEmpty()) {
results = catalog.listNamespaces();
} else {
results = catalog.listNamespaces(parent);
}

int start = INTIAL_PAGE_TOKEN.equals(pageToken) ? 0 : Integer.parseInt(pageToken);
int end = start + Integer.parseInt(pageSize);
subResults = results.subList(start, end);
String nextToken = String.valueOf(end);

if (end >= results.size()) {
nextToken = null;
}
Pair<List<Namespace>, String> page = paginate(results, pageToken, Integer.parseInt(pageSize));

return ListNamespacesResponse.builder().addAll(subResults).nextPageToken(nextToken).build();
return ListNamespacesResponse.builder()
.addAll(page.first())
.nextPageToken(page.second())
.build();
}

public static CreateNamespaceResponse createNamespace(
Expand Down Expand Up @@ -203,18 +213,11 @@ public static ListTablesResponse listTables(Catalog catalog, Namespace namespace
public static ListTablesResponse listTables(
Catalog catalog, Namespace namespace, String pageToken, String pageSize) {
List<TableIdentifier> results = catalog.listTables(namespace);
List<TableIdentifier> subResults;

int start = INTIAL_PAGE_TOKEN.equals(pageToken) ? 0 : Integer.parseInt(pageToken);
int end = start + Integer.parseInt(pageSize);
subResults = results.subList(start, end);
String nextToken = String.valueOf(end);

if (end >= results.size()) {
nextToken = null;
}
Pair<List<TableIdentifier>, String> page =
paginate(results, pageToken, Integer.parseInt(pageSize));

return ListTablesResponse.builder().addAll(subResults).nextPageToken(nextToken).build();
return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build();
}

public static LoadTableResponse stageTableCreate(
Expand Down Expand Up @@ -448,18 +451,11 @@ public static ListTablesResponse listViews(ViewCatalog catalog, Namespace namesp
public static ListTablesResponse listViews(
ViewCatalog catalog, Namespace namespace, String pageToken, String pageSize) {
List<TableIdentifier> results = catalog.listViews(namespace);
List<TableIdentifier> subResults;

int start = INTIAL_PAGE_TOKEN.equals(pageToken) ? 0 : Integer.parseInt(pageToken);
int end = start + Integer.parseInt(pageSize);
subResults = results.subList(start, end);
String nextToken = String.valueOf(end);

if (end >= results.size()) {
nextToken = null;
}
Pair<List<TableIdentifier>, String> page =
paginate(results, pageToken, Integer.parseInt(pageSize));

return ListTablesResponse.builder().addAll(subResults).nextPageToken(nextToken).build();
return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build();
}

public static LoadViewResponse createView(
Expand Down
12 changes: 6 additions & 6 deletions core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2341,13 +2341,13 @@ public void testInvalidPageSize() {
RESTSessionCatalog.REST_PAGE_SIZE));
}

@Test
public void testPaginationForListNamespaces() {
@ParameterizedTest
@ValueSource(ints = {21, 30})
public void testPaginationForListNamespaces(int numberOfItems) {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
RESTCatalog catalog =
new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config) -> adapter);
catalog.initialize("test", ImmutableMap.of(RESTSessionCatalog.REST_PAGE_SIZE, "10"));
int numberOfItems = 30;
String namespaceName = "newdb";

// create several namespaces for listing and verify
Expand Down Expand Up @@ -2403,13 +2403,13 @@ public void testPaginationForListNamespaces() {
eq(ListNamespacesResponse.class));
}

@Test
public void testPaginationForListTables() {
@ParameterizedTest
@ValueSource(ints = {21, 30})
public void testPaginationForListTables(int numberOfItems) {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
RESTCatalog catalog =
new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config) -> adapter);
catalog.initialize("test", ImmutableMap.of(RESTSessionCatalog.REST_PAGE_SIZE, "10"));
int numberOfItems = 30;
String namespaceName = "newdb";
String tableName = "newtable";
catalog.createNamespace(Namespace.of(namespaceName));
Expand Down
9 changes: 5 additions & 4 deletions core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@
import org.eclipse.jetty.servlet.ServletHolder;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mockito;

public class TestRESTViewCatalog extends ViewCatalogTests<RESTCatalog> {
Expand Down Expand Up @@ -153,14 +154,14 @@ public void closeCatalog() throws Exception {
}
}

@Test
public void testPaginationForListViews() {
@ParameterizedTest
@ValueSource(ints = {21, 30})
public void testPaginationForListViews(int numberOfItems) {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
RESTCatalog catalog =
new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config) -> adapter);
catalog.initialize("test", ImmutableMap.of(RESTSessionCatalog.REST_PAGE_SIZE, "10"));

int numberOfItems = 30;
String namespaceName = "newdb";
String viewName = "newview";

Expand Down

0 comments on commit 97c9c53

Please sign in to comment.