This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new fa66e0b5e9 [#13290] fix(iceberg): Set metadata_location when load
table with snapshots=refs (#13291)
fa66e0b5e9 is described below
commit fa66e0b5e9b7865d3f241a349b17488f6a76d1b0
Author: Xinyi Lu <[email protected]>
AuthorDate: Fri Sep 18 22:59:03 2026 -0700
[#13290] fix(iceberg): Set metadata_location when load table with
snapshots=refs (#13291)
### What changes were proposed in this pull request?
set metadata_location for TableMetadata object with lazy snapshot
loading
details in issue #13290
### Why are the changes needed?
Fix: #13290
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Unit Tests added in this PR
---
.../service/rest/IcebergTableOperations.java | 5 +-
.../service/rest/TestIcebergTableOperations.java | 85 +++++++++++++++++++++-
2 files changed, 85 insertions(+), 5 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
index 53f2ea8478..6f7fb4d80f 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
@@ -571,7 +571,10 @@ public class IcebergTableOperations {
return loadTableResponse;
}
TableMetadata filteredMetadata =
-
TableMetadata.buildFrom(metadata).suppressHistoricalSnapshots().build();
+ TableMetadata.buildFrom(metadata)
+ .withMetadataLocation(metadata.metadataFileLocation())
+ .suppressHistoricalSnapshots()
+ .build();
LoadTableResponse.Builder builder =
LoadTableResponse.builder()
.withTableMetadata(filteredMetadata)
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
index f1a45f3faa..c27d0a2f2b 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
@@ -67,10 +67,13 @@ import
org.apache.gravitino.listener.api.event.IcebergUpdateTablePreEvent;
import org.apache.gravitino.server.ServerConfig;
import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
import org.apache.iceberg.MetadataUpdate;
+import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.Snapshot;
+import org.apache.iceberg.SnapshotParser;
import org.apache.iceberg.SnapshotRef;
import org.apache.iceberg.TableMetadata;
+import org.apache.iceberg.TableMetadataParser;
import org.apache.iceberg.UpdateRequirement;
import org.apache.iceberg.UpdateRequirements;
import org.apache.iceberg.catalog.Namespace;
@@ -1091,6 +1094,25 @@ public class TestIcebergTableOperations extends
IcebergNamespaceTestBase {
refs.keySet(),
refsTableResponse.tableMetadata().refs().keySet(),
"Refs should be preserved in filtered response");
+
+ // Filtering must not alter anything other than the snapshot list
+ Assertions.assertNotNull(allTableResponse.metadataLocation());
+ Assertions.assertEquals(
+ allTableResponse.metadataLocation(),
+ refsTableResponse.metadataLocation(),
+ "snapshots=refs must keep metadata-location in the response");
+ Assertions.assertEquals(
+ allTableResponse.tableMetadata().lastUpdatedMillis(),
+ refsTableResponse.tableMetadata().lastUpdatedMillis(),
+ "snapshots=refs must not change last-updated-ms");
+ Assertions.assertEquals(
+ allTableResponse.tableMetadata().previousFiles(),
+ refsTableResponse.tableMetadata().previousFiles(),
+ "snapshots=refs must not add a metadata-log entry");
+ Assertions.assertEquals(
+ allTableResponse.tableMetadata().snapshotLog(),
+ refsTableResponse.tableMetadata().snapshotLog(),
+ "snapshots=refs must keep the full snapshot-log for lazy loading");
}
@ParameterizedTest
@@ -1133,10 +1155,7 @@ public class TestIcebergTableOperations extends
IcebergNamespaceTestBase {
void testFilterSnapshotsByRefsKeepsCredentials() {
TableMetadata metadata =
TableMetadata.newTableMetadata(
- tableSchema,
- org.apache.iceberg.PartitionSpec.unpartitioned(),
- "s3://bucket/db/tbl",
- ImmutableMap.of());
+ tableSchema, PartitionSpec.unpartitioned(), "s3://bucket/db/tbl",
ImmutableMap.of());
org.apache.iceberg.rest.credentials.Credential credential =
IcebergRESTUtils.toRESTCredential(
"s3://bucket/db/tbl/",
@@ -1160,6 +1179,64 @@ public class TestIcebergTableOperations extends
IcebergNamespaceTestBase {
Assertions.assertEquals("org.apache.iceberg.aws.s3.S3FileIO",
filtered.config().get("io-impl"));
}
+ @Test
+ void testFilterSnapshotsByRefsPreservesMetadataLocationAndHistory() {
+ TableMetadata base =
+ TableMetadata.newTableMetadata(
+ tableSchema, PartitionSpec.unpartitioned(), "s3://bucket/db/tbl",
ImmutableMap.of());
+ Snapshot first = snapshot(1L, null, 1000L);
+ Snapshot second = snapshot(2L, 1L, 2000L);
+ TableMetadata withHistory =
+ TableMetadata.buildFrom(
+ TableMetadata.buildFrom(base).setBranchSnapshot(first,
"main").build())
+ .setBranchSnapshot(second, "main")
+ .build();
+ String metadataLocation =
"s3://bucket/db/tbl/metadata/00002-abc.metadata.json";
+ // Round-trip through the parser so the metadata looks like one loaded
from a metadata file:
+ // no pending changes and a known metadata location, exactly what
loadTable hands over.
+ TableMetadata metadata =
+ TableMetadataParser.fromJson(metadataLocation,
TableMetadataParser.toJson(withHistory));
+ Assertions.assertEquals(2, metadata.snapshots().size());
+ LoadTableResponse original =
LoadTableResponse.builder().withTableMetadata(metadata).build();
+
+ LoadTableResponse filtered =
IcebergTableOperations.filterSnapshotsByRefs(original);
+
+ Assertions.assertEquals(
+ ImmutableSet.of(2L),
+ filtered.tableMetadata().snapshots().stream()
+ .map(Snapshot::snapshotId)
+ .collect(Collectors.toSet()),
+ "only the ref-referenced snapshot should remain");
+ Assertions.assertEquals(
+ metadataLocation, filtered.metadataLocation(), "metadata-location must
be preserved");
+ Assertions.assertEquals(
+ metadata.lastUpdatedMillis(),
+ filtered.tableMetadata().lastUpdatedMillis(),
+ "last-updated-ms must not be bumped by filtering");
+ Assertions.assertEquals(
+ metadata.previousFiles(),
+ filtered.tableMetadata().previousFiles(),
+ "filtering must not append a metadata-log entry");
+ Assertions.assertEquals(
+ metadata.snapshotLog(),
+ filtered.tableMetadata().snapshotLog(),
+ "snapshot-log must be kept intact for lazy snapshot loading");
+ }
+
+ private static Snapshot snapshot(long snapshotId, Long parentId, long
timestampMs) {
+ String json =
+ String.format(
+ "{\"snapshot-id\":%d,%s\"timestamp-ms\":%d,\"sequence-number\":%d,"
+ + "\"summary\":{\"operation\":\"append\"},"
+ +
"\"manifest-list\":\"s3://bucket/db/tbl/metadata/snap-%d.avro\",\"schema-id\":0}",
+ snapshotId,
+ parentId == null ? "" :
String.format("\"parent-snapshot-id\":%d,", parentId),
+ timestampMs,
+ snapshotId,
+ snapshotId);
+ return SnapshotParser.fromJson(json);
+ }
+
@ParameterizedTest
@MethodSource("org.apache.gravitino.iceberg.service.rest.IcebergRestTestUtil#testNamespaces")
void testLoadTableSnapshotsAllReturnsAllSnapshots(Namespace namespace) {
