diff --git a/core/src/main/java/org/apache/iceberg/UpdateRequirement.java b/core/src/main/java/org/apache/iceberg/UpdateRequirement.java index 80ecf84efa45..fc1f55311175 100644 --- a/core/src/main/java/org/apache/iceberg/UpdateRequirement.java +++ b/core/src/main/java/org/apache/iceberg/UpdateRequirement.java @@ -25,7 +25,10 @@ /** Represents a requirement for a {@link MetadataUpdate} */ public interface UpdateRequirement { - void validate(TableMetadata base); + default void validate(TableMetadata base) { + throw new ValidationException( + "Cannot validate %s against a table", this.getClass().getSimpleName()); + } default void validate(ViewMetadata base) { throw new ValidationException( @@ -62,12 +65,25 @@ public void validate(TableMetadata base) { "Requirement failed: UUID does not match: expected %s != %s", base.uuid(), uuid); } } + } + + class AssertViewUUID implements UpdateRequirement { + private final String uuid; + + public AssertViewUUID(String uuid) { + Preconditions.checkArgument(uuid != null, "Invalid required UUID: null"); + this.uuid = uuid; + } + + public String uuid() { + return uuid; + } @Override public void validate(ViewMetadata base) { if (!uuid.equalsIgnoreCase(base.uuid())) { throw new CommitFailedException( - "Requirement failed: UUID does not match: expected %s != %s", base.uuid(), uuid); + "Requirement failed: view UUID does not match: expected %s != %s", base.uuid(), uuid); } } } diff --git a/core/src/main/java/org/apache/iceberg/UpdateRequirementParser.java b/core/src/main/java/org/apache/iceberg/UpdateRequirementParser.java index 091d4f1fc58c..5c4dc2221290 100644 --- a/core/src/main/java/org/apache/iceberg/UpdateRequirementParser.java +++ b/core/src/main/java/org/apache/iceberg/UpdateRequirementParser.java @@ -35,6 +35,7 @@ private UpdateRequirementParser() {} // assertion types static final String ASSERT_TABLE_UUID = "assert-table-uuid"; + static final String ASSERT_VIEW_UUID = "assert-view-uuid"; static final String ASSERT_TABLE_DOES_NOT_EXIST = "assert-create"; static final String ASSERT_REF_SNAPSHOT_ID = "assert-ref-snapshot-id"; static final String ASSERT_LAST_ASSIGNED_FIELD_ID = "assert-last-assigned-field-id"; @@ -68,6 +69,7 @@ private UpdateRequirementParser() {} private static final Map, String> TYPES = ImmutableMap., String>builder() .put(UpdateRequirement.AssertTableUUID.class, ASSERT_TABLE_UUID) + .put(UpdateRequirement.AssertViewUUID.class, ASSERT_VIEW_UUID) .put(UpdateRequirement.AssertTableDoesNotExist.class, ASSERT_TABLE_DOES_NOT_EXIST) .put(UpdateRequirement.AssertRefSnapshotID.class, ASSERT_REF_SNAPSHOT_ID) .put(UpdateRequirement.AssertLastAssignedFieldId.class, ASSERT_LAST_ASSIGNED_FIELD_ID) @@ -101,6 +103,9 @@ public static void toJson(UpdateRequirement updateRequirement, JsonGenerator gen case ASSERT_TABLE_UUID: writeAssertTableUUID((UpdateRequirement.AssertTableUUID) updateRequirement, generator); break; + case ASSERT_VIEW_UUID: + writeAssertViewUUID((UpdateRequirement.AssertViewUUID) updateRequirement, generator); + break; case ASSERT_REF_SNAPSHOT_ID: writeAssertRefSnapshotId( (UpdateRequirement.AssertRefSnapshotID) updateRequirement, generator); @@ -159,6 +164,8 @@ public static UpdateRequirement fromJson(JsonNode jsonNode) { return readAssertTableDoesNotExist(jsonNode); case ASSERT_TABLE_UUID: return readAssertTableUUID(jsonNode); + case ASSERT_VIEW_UUID: + return readAssertViewUUID(jsonNode); case ASSERT_REF_SNAPSHOT_ID: return readAssertRefSnapshotId(jsonNode); case ASSERT_LAST_ASSIGNED_FIELD_ID: @@ -182,6 +189,11 @@ private static void writeAssertTableUUID( gen.writeStringField(UUID, requirement.uuid()); } + private static void writeAssertViewUUID( + UpdateRequirement.AssertViewUUID requirement, JsonGenerator gen) throws IOException { + gen.writeStringField(UUID, requirement.uuid()); + } + private static void writeAssertRefSnapshotId( UpdateRequirement.AssertRefSnapshotID requirement, JsonGenerator gen) throws IOException { gen.writeStringField(NAME, requirement.refName()); @@ -231,6 +243,11 @@ private static UpdateRequirement readAssertTableUUID(JsonNode node) { return new UpdateRequirement.AssertTableUUID(uuid); } + private static UpdateRequirement readAssertViewUUID(JsonNode node) { + String uuid = JsonUtil.getString(UUID, node); + return new UpdateRequirement.AssertViewUUID(uuid); + } + private static UpdateRequirement readAssertRefSnapshotId(JsonNode node) { String name = JsonUtil.getString(NAME, node); Long snapshotId = JsonUtil.getLongOrNull(SNAPSHOT_ID, node); diff --git a/core/src/main/java/org/apache/iceberg/UpdateRequirements.java b/core/src/main/java/org/apache/iceberg/UpdateRequirements.java index 8a7a761ff2c1..6a5d07d7813d 100644 --- a/core/src/main/java/org/apache/iceberg/UpdateRequirements.java +++ b/core/src/main/java/org/apache/iceberg/UpdateRequirements.java @@ -23,6 +23,7 @@ import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.Sets; +import org.apache.iceberg.view.ViewMetadata; public class UpdateRequirements { @@ -56,6 +57,16 @@ public static List forUpdateTable( return builder.build(); } + public static List forReplaceView( + ViewMetadata base, List metadataUpdates) { + Preconditions.checkArgument(null != base, "Invalid view metadata: null"); + Preconditions.checkArgument(null != metadataUpdates, "Invalid metadata updates: null"); + Builder builder = new Builder(null, false); + builder.require(new UpdateRequirement.AssertViewUUID(base.uuid())); + metadataUpdates.forEach(builder::update); + return builder.build(); + } + private static class Builder { private final TableMetadata base; private final ImmutableList.Builder requirements = ImmutableList.builder(); diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTViewOperations.java b/core/src/main/java/org/apache/iceberg/rest/RESTViewOperations.java index 48dc075b1305..b4dafaa9031b 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTViewOperations.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTViewOperations.java @@ -21,9 +21,8 @@ import java.util.Map; import java.util.Objects; import java.util.function.Supplier; -import org.apache.iceberg.UpdateRequirement; +import org.apache.iceberg.UpdateRequirements; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; -import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.rest.requests.UpdateTableRequest; import org.apache.iceberg.rest.responses.LoadViewResponse; import org.apache.iceberg.view.ViewMetadata; @@ -62,9 +61,7 @@ public void commit(ViewMetadata base, ViewMetadata metadata) { UpdateTableRequest request = UpdateTableRequest.create( - null, - ImmutableList.of(new UpdateRequirement.AssertTableUUID(base.uuid())), - metadata.changes()); + null, UpdateRequirements.forReplaceView(base, metadata.changes()), metadata.changes()); LoadViewResponse response = client.post( diff --git a/core/src/test/java/org/apache/iceberg/TestUpdateRequirementParser.java b/core/src/test/java/org/apache/iceberg/TestUpdateRequirementParser.java index 92401b9d9ef5..cd32b9606d55 100644 --- a/core/src/test/java/org/apache/iceberg/TestUpdateRequirementParser.java +++ b/core/src/test/java/org/apache/iceberg/TestUpdateRequirementParser.java @@ -58,6 +58,25 @@ public void testAssertUUIDToJson() { .isEqualTo(expected); } + @Test + public void testAssertViewUUIDFromJson() { + String requirementType = UpdateRequirementParser.ASSERT_VIEW_UUID; + String uuid = "2cc52516-5e73-41f2-b139-545d41a4e151"; + String json = String.format("{\"type\":\"assert-view-uuid\",\"uuid\":\"%s\"}", uuid); + UpdateRequirement expected = new UpdateRequirement.AssertViewUUID(uuid); + assertEquals(requirementType, expected, UpdateRequirementParser.fromJson(json)); + } + + @Test + public void testAssertViewUUIDToJson() { + String uuid = "2cc52516-5e73-41f2-b139-545d41a4e151"; + String expected = String.format("{\"type\":\"assert-view-uuid\",\"uuid\":\"%s\"}", uuid); + UpdateRequirement actual = new UpdateRequirement.AssertViewUUID(uuid); + Assertions.assertThat(UpdateRequirementParser.toJson(actual)) + .as("AssertViewUUID should convert to the correct JSON value") + .isEqualTo(expected); + } + @Test public void testAssertTableDoesNotExistFromJson() { String requirementType = UpdateRequirementParser.ASSERT_TABLE_DOES_NOT_EXIST; @@ -262,6 +281,10 @@ public void assertEquals( (UpdateRequirement.AssertTableUUID) expected, (UpdateRequirement.AssertTableUUID) actual); break; + case UpdateRequirementParser.ASSERT_VIEW_UUID: + compareAssertViewUUID( + (UpdateRequirement.AssertViewUUID) expected, (UpdateRequirement.AssertViewUUID) actual); + break; case UpdateRequirementParser.ASSERT_TABLE_DOES_NOT_EXIST: // Don't cast here as the function explicitly tests that the types are correct, given that // the generated JSON @@ -312,6 +335,15 @@ private static void compareAssertTableUUID( .isEqualTo(expected.uuid()); } + private static void compareAssertViewUUID( + UpdateRequirement.AssertViewUUID expected, UpdateRequirement.AssertViewUUID actual) { + Assertions.assertThat(actual.uuid()) + .as("UUID from JSON should not be null") + .isNotNull() + .as("UUID should parse correctly from JSON") + .isEqualTo(expected.uuid()); + } + // AssertTableDoesNotExist does not have any fields beyond the requirement type, so just check // that the classes // are the same and as expected. diff --git a/core/src/test/java/org/apache/iceberg/TestUpdateRequirements.java b/core/src/test/java/org/apache/iceberg/TestUpdateRequirements.java index bf3c32628ab0..ed1142441736 100644 --- a/core/src/test/java/org/apache/iceberg/TestUpdateRequirements.java +++ b/core/src/test/java/org/apache/iceberg/TestUpdateRequirements.java @@ -26,11 +26,14 @@ import java.util.List; import java.util.UUID; +import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.exceptions.CommitFailedException; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Sets; import org.apache.iceberg.types.Types; +import org.apache.iceberg.view.ImmutableViewVersion; +import org.apache.iceberg.view.ViewMetadata; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -38,12 +41,16 @@ public class TestUpdateRequirements { private final TableMetadata metadata = mock(TableMetadata.class); private final TableMetadata updated = mock(TableMetadata.class); + private final ViewMetadata viewMetadata = mock(ViewMetadata.class); + private final ViewMetadata updatedViewMetadata = mock(ViewMetadata.class); @BeforeEach public void before() { String uuid = UUID.randomUUID().toString(); when(metadata.uuid()).thenReturn(uuid); when(updated.uuid()).thenReturn(uuid); + when(viewMetadata.uuid()).thenReturn(uuid); + when(updatedViewMetadata.uuid()).thenReturn(uuid); } @Test @@ -67,6 +74,14 @@ public void nullCheck() { assertThatThrownBy(() -> UpdateRequirements.forReplaceTable(metadata, null)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Invalid metadata updates: null"); + + assertThatThrownBy(() -> UpdateRequirements.forReplaceView(null, null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid view metadata: null"); + + assertThatThrownBy(() -> UpdateRequirements.forReplaceView(viewMetadata, null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid metadata updates: null"); } @Test @@ -87,6 +102,13 @@ public void emptyUpdatesForUpdateAndReplaceTable() { .hasOnlyElementsOfType(UpdateRequirement.AssertTableUUID.class); } + @Test + public void emptyUpdatesForReplaceView() { + assertThat(UpdateRequirements.forReplaceView(viewMetadata, ImmutableList.of())) + .hasSize(1) + .hasOnlyElementsOfType(UpdateRequirement.AssertViewUUID.class); + } + @Test public void tableAlreadyExists() { List requirements = UpdateRequirements.forCreateTable(ImmutableList.of()); @@ -129,6 +151,39 @@ public void assignUUIDFailure() { updated.uuid(), metadata.uuid())); } + @Test + public void assignUUIDToView() { + List requirements = + UpdateRequirements.forReplaceView( + viewMetadata, + ImmutableList.of( + new MetadataUpdate.AssignUUID(viewMetadata.uuid()), + new MetadataUpdate.AssignUUID(UUID.randomUUID().toString()), + new MetadataUpdate.AssignUUID(UUID.randomUUID().toString()))); + requirements.forEach(req -> req.validate(viewMetadata)); + + assertThat(requirements) + .hasSize(1) + .hasOnlyElementsOfType(UpdateRequirement.AssertViewUUID.class); + + assertViewUUID(requirements); + } + + @Test + public void assignUUIDToViewFailure() { + List requirements = + UpdateRequirements.forReplaceView( + viewMetadata, ImmutableList.of(new MetadataUpdate.AssignUUID(viewMetadata.uuid()))); + + when(updatedViewMetadata.uuid()).thenReturn(UUID.randomUUID().toString()); + assertThatThrownBy(() -> requirements.forEach(req -> req.validate(updatedViewMetadata))) + .isInstanceOf(CommitFailedException.class) + .hasMessage( + String.format( + "Requirement failed: view UUID does not match: expected %s != %s", + updatedViewMetadata.uuid(), viewMetadata.uuid())); + } + @Test public void upgradeFormatVersion() { List requirements = @@ -143,6 +198,20 @@ public void upgradeFormatVersion() { assertTableUUID(requirements); } + @Test + public void upgradeFormatVersionForView() { + List requirements = + UpdateRequirements.forReplaceView( + viewMetadata, ImmutableList.of(new MetadataUpdate.UpgradeFormatVersion(2))); + requirements.forEach(req -> req.validate(viewMetadata)); + + assertThat(requirements) + .hasSize(1) + .hasOnlyElementsOfType(UpdateRequirement.AssertViewUUID.class); + + assertViewUUID(requirements); + } + @Test public void addSchema() { int lastColumnId = 1; @@ -190,6 +259,25 @@ public void addSchemaFailure() { .hasMessage("Requirement failed: last assigned field id changed: expected id 2 != 3"); } + @Test + public void addSchemaForView() { + int lastColumnId = 1; + List requirements = + UpdateRequirements.forReplaceView( + viewMetadata, + ImmutableList.of( + new MetadataUpdate.AddSchema(new Schema(), lastColumnId), + new MetadataUpdate.AddSchema(new Schema(), lastColumnId + 1), + new MetadataUpdate.AddSchema(new Schema(), lastColumnId + 2))); + requirements.forEach(req -> req.validate(viewMetadata)); + + assertThat(requirements) + .hasSize(1) + .hasOnlyElementsOfTypes(UpdateRequirement.AssertViewUUID.class); + + assertViewUUID(requirements); + } + @Test public void setCurrentSchema() { int schemaId = 3; @@ -553,6 +641,33 @@ public void setAndRemoveProperties() { assertTableUUID(requirements); } + @Test + public void setAndRemovePropertiesForView() { + List requirements = + UpdateRequirements.forReplaceView( + viewMetadata, + ImmutableList.of(new MetadataUpdate.SetProperties(ImmutableMap.of("test", "test")))); + requirements.forEach(req -> req.validate(viewMetadata)); + + assertThat(requirements) + .hasSize(1) + .hasOnlyElementsOfTypes(UpdateRequirement.AssertViewUUID.class); + + assertViewUUID(requirements); + + requirements = + UpdateRequirements.forReplaceView( + viewMetadata, + ImmutableList.of(new MetadataUpdate.RemoveProperties(Sets.newHashSet("test")))); + requirements.forEach(req -> req.validate(viewMetadata)); + + assertThat(requirements) + .hasSize(1) + .hasOnlyElementsOfTypes(UpdateRequirement.AssertViewUUID.class); + + assertViewUUID(requirements); + } + @Test public void setLocation() { List requirements = @@ -567,6 +682,93 @@ public void setLocation() { assertTableUUID(requirements); } + @Test + public void setLocationForView() { + List requirements = + UpdateRequirements.forReplaceView( + viewMetadata, ImmutableList.of(new MetadataUpdate.SetLocation("location"))); + requirements.forEach(req -> req.validate(viewMetadata)); + + assertThat(requirements) + .hasSize(1) + .hasOnlyElementsOfTypes(UpdateRequirement.AssertViewUUID.class); + + assertViewUUID(requirements); + } + + @Test + public void addViewVersion() { + List requirements = + UpdateRequirements.forReplaceView( + viewMetadata, + ImmutableList.of( + new MetadataUpdate.AddViewVersion( + ImmutableViewVersion.builder() + .versionId(1) + .schemaId(1) + .timestampMillis(System.currentTimeMillis()) + .defaultNamespace(Namespace.of("ns")) + .build()), + new MetadataUpdate.AddViewVersion( + ImmutableViewVersion.builder() + .versionId(2) + .schemaId(1) + .timestampMillis(System.currentTimeMillis()) + .defaultNamespace(Namespace.of("ns")) + .build()), + new MetadataUpdate.AddViewVersion( + ImmutableViewVersion.builder() + .versionId(3) + .schemaId(1) + .timestampMillis(System.currentTimeMillis()) + .defaultNamespace(Namespace.of("ns")) + .build()))); + requirements.forEach(req -> req.validate(viewMetadata)); + + assertThat(requirements) + .hasSize(1) + .hasOnlyElementsOfTypes(UpdateRequirement.AssertViewUUID.class); + + assertViewUUID(requirements); + } + + @Test + public void setCurrentViewVersion() { + List requirements = + UpdateRequirements.forReplaceView( + viewMetadata, + ImmutableList.of( + new MetadataUpdate.AddViewVersion( + ImmutableViewVersion.builder() + .versionId(3) + .schemaId(1) + .timestampMillis(System.currentTimeMillis()) + .defaultNamespace(Namespace.of("ns")) + .build()), + new MetadataUpdate.AddViewVersion( + ImmutableViewVersion.builder() + .versionId(2) + .schemaId(1) + .timestampMillis(System.currentTimeMillis()) + .defaultNamespace(Namespace.of("ns")) + .build()), + new MetadataUpdate.AddViewVersion( + ImmutableViewVersion.builder() + .versionId(1) + .schemaId(1) + .timestampMillis(System.currentTimeMillis()) + .defaultNamespace(Namespace.of("ns")) + .build()), + new MetadataUpdate.SetCurrentViewVersion(2))); + requirements.forEach(req -> req.validate(viewMetadata)); + + assertThat(requirements) + .hasSize(1) + .hasOnlyElementsOfTypes(UpdateRequirement.AssertViewUUID.class); + + assertViewUUID(requirements); + } + private void assertTableUUID(List requirements) { assertThat(requirements) .element(0) @@ -574,4 +776,12 @@ private void assertTableUUID(List requirements) { .extracting(UpdateRequirement.AssertTableUUID::uuid) .isEqualTo(metadata.uuid()); } + + private void assertViewUUID(List requirements) { + assertThat(requirements) + .element(0) + .asInstanceOf(InstanceOfAssertFactories.type(UpdateRequirement.AssertViewUUID.class)) + .extracting(UpdateRequirement.AssertViewUUID::uuid) + .isEqualTo(viewMetadata.uuid()); + } } diff --git a/open-api/rest-catalog-open-api.py b/open-api/rest-catalog-open-api.py index 5a17b9d43856..6cd60fe9abd0 100644 --- a/open-api/rest-catalog-open-api.py +++ b/open-api/rest-catalog-open-api.py @@ -420,6 +420,10 @@ class AssertDefaultSortOrderId(TableRequirement): default_sort_order_id: int = Field(..., alias='default-sort-order-id') +class ViewRequirement(BaseModel): + __root__: Any = Field(..., discriminator='type') + + class RegisterTableRequest(BaseModel): name: str metadata_location: str = Field(..., alias='metadata-location') @@ -822,6 +826,7 @@ class CommitViewRequest(BaseModel): identifier: Optional[TableIdentifier] = Field( None, description='View identifier to update' ) + requirements: Optional[List[ViewRequirement]] = None updates: List[ViewUpdate] diff --git a/open-api/rest-catalog-open-api.yaml b/open-api/rest-catalog-open-api.yaml index c965793cb229..a9d30ed02c63 100644 --- a/open-api/rest-catalog-open-api.yaml +++ b/open-api/rest-catalog-open-api.yaml @@ -2510,6 +2510,32 @@ components: default-sort-order-id: type: integer + ViewRequirement: + discriminator: + propertyName: type + mapping: + assert-view-uuid: '#/components/schemas/AssertViewUUID' + type: object + required: + - type + properties: + type: + type: "string" + + AssertViewUUID: + allOf: + - $ref: "#/components/schemas/ViewRequirement" + description: The view UUID must match the requirement's `uuid` + required: + - type + - uuid + properties: + type: + type: string + enum: [ "assert-view-uuid" ] + uuid: + type: string + LoadTableResult: description: | Result used when a table is successfully loaded. @@ -2576,6 +2602,10 @@ components: identifier: description: View identifier to update $ref: '#/components/schemas/TableIdentifier' + requirements: + type: array + items: + $ref: '#/components/schemas/ViewRequirement' updates: type: array items: