singhpk234 commented on code in PR #1405:
URL: https://github.com/apache/polaris/pull/1405#discussion_r2051279792
##########
service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1010,6 +1028,31 @@ public void renameView(RenameTableRequest request) {
CatalogHandlers.renameView(viewCatalog, request);
}
+ private @Nonnull LoadTableResponse filterResponseToSnapshots(
+ LoadTableResponse loadTableResponse, String snapshots) {
+ if (snapshots == null || snapshots.equalsIgnoreCase(SNAPSHOTS_ALL)) {
+ return loadTableResponse;
+ } else if (snapshots.equalsIgnoreCase(SNAPSHOTS_REFS)) {
+ TableMetadata metadata = loadTableResponse.tableMetadata();
+
+ Set<Long> referencedSnapshotIds =
+ metadata.refs().values().stream()
+ .map(SnapshotRef::snapshotId)
+ .collect(Collectors.toSet());
+
+ TableMetadata filteredMetadata =
+ metadata.removeSnapshotsIf(s ->
referencedSnapshotIds.contains(s.snapshotId()));
Review Comment:
[doubt] I thought we want to return the snapshots that are being referenced
? should this be a
```suggestion
TableMetadata filteredMetadata =
metadata.removeSnapshotsIf(s -> !
referencedSnapshotIds.contains(s.snapshotId()));
```
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java:
##########
@@ -1343,4 +1346,67 @@ public void testDropNonExistingGenericTable() {
genericTableApi.purge(currentCatalogName, namespace);
}
+
+ @Test
+ public void testLoadTableWithSnapshots() {
+ Namespace namespace = Namespace.of("ns1");
+ restCatalog.createNamespace(namespace);
+ TableIdentifier tableIdentifier = TableIdentifier.of(namespace, "tbl1");
+ restCatalog.createTable(tableIdentifier, SCHEMA);
+
+ assertThat(catalogApi.loadTable(currentCatalogName, tableIdentifier,
"ALL"))
+ .isEqualTo(OK.getStatusCode());
+ assertThat(catalogApi.loadTable(currentCatalogName, tableIdentifier,
"all"))
+ .isEqualTo(OK.getStatusCode());
+ assertThat(catalogApi.loadTable(currentCatalogName, tableIdentifier,
"refs"))
+ .isEqualTo(OK.getStatusCode());
+ assertThat(catalogApi.loadTable(currentCatalogName, tableIdentifier,
"REFS"))
+ .isEqualTo(OK.getStatusCode());
+ assertThat(catalogApi.loadTable(currentCatalogName, tableIdentifier,
"not-real"))
+ .isEqualTo(BAD_REQUEST.getStatusCode());
+
+ catalogApi.purge(currentCatalogName, namespace);
+ }
+
+ @Test
+ public void testLoadTableWithRefFiltering() {
+ Namespace namespace = Namespace.of("ns1");
+ restCatalog.createNamespace(namespace);
+ TableIdentifier tableIdentifier = TableIdentifier.of(namespace, "tbl1");
+
+ restCatalog.createTable(tableIdentifier, SCHEMA);
+
+ Table table = restCatalog.loadTable(tableIdentifier);
+
+ // Create an orphaned snapshot:
+ table.newAppend().appendFile(FILE_A).commit();
+ long snapshotIdA = table.currentSnapshot().snapshotId();
+ table.newAppend().appendFile(FILE_B).commit();
+ table.manageSnapshots().setCurrentSnapshot(snapshotIdA).commit();
+
+ String ns = RESTUtil.encodeNamespace(tableIdentifier.namespace());
+ try (Response res =
+ catalogApi
+ .request(
+ "v1/{cat}/namespaces/" + ns + "/tables/{table}",
+ Map.of("cat", currentCatalogName, "table",
tableIdentifier.name()),
+ Map.of("snapshots", "all"))
+ .get()) {
+ LoadTableResponse responseContent =
res.readEntity(LoadTableResponse.class);
+
assertThat(responseContent.tableMetadata().snapshots().size()).isEqualTo(2);
+ }
+
+ try (Response res =
+ catalogApi
+ .request(
+ "v1/{cat}/namespaces/" + ns + "/tables/{table}",
+ Map.of("cat", currentCatalogName, "table",
tableIdentifier.name()),
+ Map.of("snapshots", "refs"))
+ .get()) {
+ LoadTableResponse responseContent =
res.readEntity(LoadTableResponse.class);
+
assertThat(responseContent.tableMetadata().snapshots().size()).isEqualTo(1);
Review Comment:
can we also assert the intended snapshot id ? as if we got back the
snapshot-id we wanted ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
