This is an automated email from the ASF dual-hosted git repository.
jshao 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 d059a81933 [#9285] fix(iceberg): fix TestIcebergTableOperations
failure (#9286)
d059a81933 is described below
commit d059a81933d34237156436b704c1c3c428cfa28f
Author: FANNG <[email protected]>
AuthorDate: Fri Nov 28 13:54:25 2025 +0900
[#9285] fix(iceberg): fix TestIcebergTableOperations failure (#9286)
### What changes were proposed in this pull request?
Remove the check for start snapshot id < end snapshot id, because the
snapshot id is random. see:
https://github.com/apache/iceberg/blob/42719ef41eefc56968b528c51550f3ac63682eb2/core/src/main/java/org/apache/iceberg/SnapshotIdGeneratorUtil.java#L32-L37
### Why are the changes needed?
Fix: #9285
### Does this PR introduce _any_ user-facing change?
no
### How was this patch tested?
run the test multiple times.
---
.../iceberg/service/CatalogWrapperForREST.java | 9 ++---
.../service/rest/TestIcebergTableOperations.java | 38 ++--------------------
2 files changed, 5 insertions(+), 42 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
index b24303a453..2c96cfd5f8 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
@@ -345,12 +345,6 @@ public class CatalogWrapperForREST extends
IcebergCatalogWrapper {
Long endSnapshotId = scanRequest.endSnapshotId();
// Use IncrementalAppendScan if both start and end snapshot IDs are
provided
if (startSnapshotId != null && endSnapshotId != null) {
- if (startSnapshotId >= endSnapshotId) {
- throw new IllegalArgumentException(
- String.format(
- "Invalid snapshot range: startSnapshotId (%d) must be less
than endSnapshotId (%d)",
- startSnapshotId, endSnapshotId));
- }
LOG.debug(
"Using IncrementalAppendScan for table: {}, from snapshot: {} to
snapshot: {}",
tableIdentifier,
@@ -365,7 +359,8 @@ public class CatalogWrapperForREST extends
IcebergCatalogWrapper {
return incrementalScan.planFiles();
} else {
TableScan tableScan = table.newScan();
- if (scanRequest.snapshotId() != null && scanRequest.snapshotId() != 0L) {
+ // Snapshot ID 0 has no special meaning in Iceberg, so we only apply if
not null
+ if (scanRequest.snapshotId() != null) {
tableScan = tableScan.useSnapshot(scanRequest.snapshotId());
LOG.debug("Applied snapshot filter: snapshot-id={}",
scanRequest.snapshotId());
}
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 848d78f451..a41cb86341 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
@@ -199,25 +199,6 @@ public class TestIcebergTableOperations extends
IcebergNamespaceTestBase {
dummyEventListener.popPostEvent() instanceof
IcebergPlanTableScanFailureEvent);
}
- @ParameterizedTest
-
@MethodSource("org.apache.gravitino.iceberg.service.rest.IcebergRestTestUtil#testNamespaces")
- void testPlanTableScanWithIncrementalAppendScan(Namespace namespace) {
- verifyCreateNamespaceSucc(namespace);
- verifyCreateTableSucc(namespace, "incremental_scan_table", true);
-
- dummyEventListener.clearEvent();
- TableMetadata metadata = getTableMeta(namespace, "incremental_scan_table");
- Long currentSnapshotId = metadata.currentSnapshot().snapshotId();
-
- PlanTableScanRequest invalidRequest =
- buildPlanTableScanRequestWithRange(currentSnapshotId,
currentSnapshotId);
- Response response = doPlanTableScan(namespace, "incremental_scan_table",
invalidRequest);
- Assertions.assertEquals(
- Status.BAD_REQUEST.getStatusCode(),
- response.getStatus(),
- "Expected BAD_REQUEST for start == end snapshot IDs");
- }
-
@ParameterizedTest
@MethodSource("org.apache.gravitino.iceberg.service.rest.IcebergRestTestUtil#testNamespaces")
void testPlanTableScanWithIncrementalAppendScanValidRange(Namespace
namespace) {
@@ -233,25 +214,12 @@ public class TestIcebergTableOperations extends
IcebergNamespaceTestBase {
Assertions.assertNotNull(snapshots, "Snapshots should not be null");
Assertions.assertTrue(snapshots.size() >= 2, "Should have at least 2
snapshots");
- // Sort by sequence number to get ordered snapshots
- List<Snapshot> sortedSnapshots =
- snapshots.stream()
- .sorted((s1, s2) -> Long.compare(s1.snapshotId(), s2.snapshotId()))
- .collect(java.util.stream.Collectors.toList());
-
- Assertions.assertTrue(
- sortedSnapshots.size() >= 2,
- "Should have at least 2 snapshots for incremental scan test, but got: "
- + sortedSnapshots.size());
-
- // For IncrementalAppendScan, start snapshot must be an ancestor of end
snapshot
- // Use the last snapshot as end, and find its parent from the sorted list
- Snapshot endSnapshot = sortedSnapshots.get(sortedSnapshots.size() - 1);
+ Snapshot endSnapshot = snapshots.get(snapshots.size() - 1);
Long endSnapshotId = endSnapshot.snapshotId();
Long startSnapshotId = endSnapshot.parentId();
-
Assertions.assertNotNull(
- startSnapshotId, "Could not find a valid startSnapshotId from
snapshots");
+ startSnapshotId, "End snapshot should have a parent snapshot for
incremental scan");
+
JsonNode planResponse =
verifyPlanTableScanSuccWithRange(
namespace, "incremental_scan_valid_table", startSnapshotId,
endSnapshotId);
