From afe554a36ef86cee068e737b9cbef6630ede8199 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Mon, 9 Dec 2024 12:21:39 +0100 Subject: [PATCH] Fix update-requirement check to handle "no current snapshot" requirement properly The current implementation of the `assert-ref-snapshot-id` update requirement neglects the `null` requirement case, which means "there must be no current snapshot". The fix is simple. Also added a check that the ref-name in that requirement-check is `main`. Also updated a few cases to eliminate IDE warnings in the same class. Fixes #10062 --- CHANGELOG.md | 2 ++ .../iceberg/rest/IcebergMetadataUpdate.java | 12 ++++++++ .../rest/IcebergUpdateRequirement.java | 29 ++++++++++++++----- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c08c43362f..c8464919f13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ as necessary. Empty sections will not end in the release notes. ### Fixes +- Fix handling of Iceberg update-requirement "no current snapshot" + ### Commits ## [0.101.0] Release (2024-12-06) diff --git a/catalog/format/iceberg/src/main/java/org/projectnessie/catalog/formats/iceberg/rest/IcebergMetadataUpdate.java b/catalog/format/iceberg/src/main/java/org/projectnessie/catalog/formats/iceberg/rest/IcebergMetadataUpdate.java index 63a95fd1ab0..527a864d07b 100644 --- a/catalog/format/iceberg/src/main/java/org/projectnessie/catalog/formats/iceberg/rest/IcebergMetadataUpdate.java +++ b/catalog/format/iceberg/src/main/java/org/projectnessie/catalog/formats/iceberg/rest/IcebergMetadataUpdate.java @@ -702,6 +702,15 @@ interface SetSnapshotRef extends IcebergMetadataUpdate { @Override default void applyToTable(IcebergTableMetadataUpdateState state) { + checkState( + "main".equals(refName()) && "branch".equals(type()), + "Nessie only supports the current snapshot-ref 'main', use Nessie's branches instead"); + var currentSnapshotId = state.snapshot().icebergSnapshotId(); + checkState( + Objects.equals(snapshotId(), state.snapshot().icebergSnapshotId()), + "Snapshot ID mismatch, must be %s, but is %s", + currentSnapshotId, + snapshotId()); state.addCatalogOp(CatalogOps.META_SET_SNAPSHOT_REF); // NOP - This class is used for JSON deserialization only. // Nessie has catalog-level branches and tags. @@ -718,6 +727,9 @@ interface RemoveSnapshotRef extends IcebergMetadataUpdate { @Override default void applyToTable(IcebergTableMetadataUpdateState state) { + checkState( + "main".equals(refName()), + "Nessie only supports the Iceberg snapshot-ref 'main', use Nessie's branches instead"); state.addCatalogOp(CatalogOps.META_REMOVE_SNAPSHOT_REF); // NOP - This class is used for JSON deserialization only. // Nessie has catalog-level branches and tags. diff --git a/catalog/format/iceberg/src/main/java/org/projectnessie/catalog/formats/iceberg/rest/IcebergUpdateRequirement.java b/catalog/format/iceberg/src/main/java/org/projectnessie/catalog/formats/iceberg/rest/IcebergUpdateRequirement.java index e94347a38d2..99cc7d122d2 100644 --- a/catalog/format/iceberg/src/main/java/org/projectnessie/catalog/formats/iceberg/rest/IcebergUpdateRequirement.java +++ b/catalog/format/iceberg/src/main/java/org/projectnessie/catalog/formats/iceberg/rest/IcebergUpdateRequirement.java @@ -16,6 +16,7 @@ package org.projectnessie.catalog.formats.iceberg.rest; import static java.lang.String.format; +import static org.projectnessie.catalog.formats.iceberg.meta.IcebergTableMetadata.NO_SNAPSHOT_ID; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonSubTypes; @@ -96,7 +97,7 @@ interface AssertUUID extends IcebergUpdateRequirement { default void check(ContentKey key, NessieEntitySnapshot snapshot) { String tableUuid = snapshot.entity().icebergUuid(); checkState( - uuid().equals(tableUuid), + Objects.equals(uuid(), tableUuid), key, "Requirement failed: UUID does not match: expected %s != %s", tableUuid, @@ -172,13 +173,27 @@ default void checkForTable( NessieTableSnapshot snapshot, boolean tableExists, ContentKey contentKey) { // Cannot really check the reference name, because the ref-name in a table-metadata is // something very different from Nessie references - Long id = snapshotId(); - if (id != null) { + checkState( + "main".equals(ref()), + contentKey, + "Requirement failed: ref must be 'main', but is '%s'", + ref()); + + Long expectedId = snapshotId(); + Long currentId = snapshot.icebergSnapshotId(); + if (expectedId != null) { checkState( - Objects.equals(id, snapshot.icebergSnapshotId()), + Objects.equals(expectedId, currentId), contentKey, "Requirement failed: snapshot id changed: expected %s != %s", - id, + expectedId, + snapshotId()); + } else { + // 'null' means "no current snapshot" + checkState( + currentId == null || currentId == NO_SNAPSHOT_ID, + contentKey, + "Requirement failed: snapshot id mismatch: expected no current snapshot, but has %s", snapshotId()); } } @@ -196,7 +211,7 @@ default void checkForTable( NessieTableSnapshot snapshot, boolean tableExists, ContentKey contentKey) { Integer id = snapshot.icebergLastColumnId(); checkState( - lastAssignedFieldId() == id, + Objects.equals(id, lastAssignedFieldId()), contentKey, "Requirement failed: last assigned field id changed: expected %s != %s", id, @@ -246,7 +261,7 @@ default void checkForTable( NessieTableSnapshot snapshot, boolean tableExists, ContentKey contentKey) { Integer id = snapshot.icebergLastPartitionId(); checkState( - lastAssignedPartitionId() == id, + Objects.equals(lastAssignedPartitionId(), id), contentKey, "Requirement failed: last assigned partition id changed: expected %s != %s", id,