diff --git a/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java b/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java index 280fe7565d75..aeb434bf0d5e 100644 --- a/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java +++ b/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java @@ -70,7 +70,6 @@ private MetadataUpdateParser() {} // AddSchema private static final String SCHEMA = "schema"; - private static final String LAST_COLUMN_ID = "last-column-id"; // SetCurrentSchema private static final String SCHEMA_ID = "schema-id"; @@ -370,7 +369,6 @@ private static void writeAddSchema(MetadataUpdate.AddSchema update, JsonGenerato throws IOException { gen.writeFieldName(SCHEMA); SchemaParser.toJson(update.schema(), gen); - gen.writeNumberField(LAST_COLUMN_ID, update.lastColumnId()); } private static void writeSetCurrentSchema( diff --git a/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java b/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java index c661ac834d45..32ba08868c01 100644 --- a/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java +++ b/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java @@ -124,14 +124,23 @@ public void testAddSchemaFromJson() { assertEquals(action, actualUpdate, MetadataUpdateParser.fromJson(json)); } + @Test + public void testAddSchemaFromJsonWithDeprecatedLastColumnId() { + String action = MetadataUpdateParser.ADD_SCHEMA; + Schema schema = ID_DATA_SCHEMA; + String json = + String.format( + "{\"action\":\"add-schema\",\"schema\":%s,\"last-column-id\":34}", + SchemaParser.toJson(schema)); + MetadataUpdate actualUpdate = new MetadataUpdate.AddSchema(schema); + assertEquals(action, actualUpdate, MetadataUpdateParser.fromJson(json)); + } + @Test public void testAddSchemaToJson() { Schema schema = ID_DATA_SCHEMA; - int lastColumnId = schema.highestFieldId(); String expected = - String.format( - "{\"action\":\"add-schema\",\"schema\":%s,\"last-column-id\":%d}", - SchemaParser.toJson(schema), lastColumnId); + String.format("{\"action\":\"add-schema\",\"schema\":%s}", SchemaParser.toJson(schema)); MetadataUpdate update = new MetadataUpdate.AddSchema(schema); String actual = MetadataUpdateParser.toJson(update); assertThat(actual) diff --git a/core/src/test/java/org/apache/iceberg/rest/TestCatalogHandlers.java b/core/src/test/java/org/apache/iceberg/rest/TestCatalogHandlers.java new file mode 100644 index 000000000000..2fa9756ff4fc --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/rest/TestCatalogHandlers.java @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.rest; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.nio.file.Path; +import java.util.List; +import org.apache.iceberg.MetadataUpdate; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.SortOrder; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.TestTables; +import org.apache.iceberg.UpdateRequirement; +import org.apache.iceberg.UpdateRequirements; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; +import org.apache.iceberg.rest.requests.UpdateTableRequest; +import org.apache.iceberg.rest.requests.UpdateTableRequestParser; +import org.apache.iceberg.types.Types; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +public class TestCatalogHandlers { + private static final int LAST_ADDED = -1; + + @TempDir private Path temp; + + @AfterEach + public void clearTables() { + TestTables.clearTables(); + } + + @Test + public void commitAddSchemaWithoutDeprecatedLastColumnIdPreservesTableLastColumnId() { + Schema baseSchema = + new Schema( + Types.NestedField.required(1, "id", Types.LongType.get()), + Types.NestedField.optional(2, "dropme", Types.StringType.get())); + TestTables.TestTableOperations ops = new TestTables.TestTableOperations("test", temp.toFile()); + TestTables.create( + temp.toFile(), + "test", + baseSchema, + PartitionSpec.unpartitioned(), + SortOrder.unsorted(), + 2, + ops); + TableMetadata base = ops.current(); + assertThat(base.lastColumnId()).isEqualTo(2); + + Schema schemaAfterDroppingHighestId = + new Schema(Types.NestedField.required(1, "id", Types.LongType.get())); + List updates = + ImmutableList.of( + new MetadataUpdate.AddSchema(schemaAfterDroppingHighestId), + new MetadataUpdate.SetCurrentSchema(LAST_ADDED)); + List requirements = UpdateRequirements.forUpdateTable(base, updates); + UpdateTableRequest request = new UpdateTableRequest(requirements, updates); + + String json = UpdateTableRequestParser.toJson(request, true); + assertThat(json).doesNotContain("last-column-id"); + + TableMetadata updated = CatalogHandlers.commit(ops, UpdateTableRequestParser.fromJson(json)); + + assertThat(updated.schema().highestFieldId()).isEqualTo(1); + assertThat(updated.lastColumnId()).isEqualTo(2); + } +} diff --git a/core/src/test/java/org/apache/iceberg/rest/requests/TestUpdateTableRequestParser.java b/core/src/test/java/org/apache/iceberg/rest/requests/TestUpdateTableRequestParser.java index 9522801cadd2..d92046677ede 100644 --- a/core/src/test/java/org/apache/iceberg/rest/requests/TestUpdateTableRequestParser.java +++ b/core/src/test/java/org/apache/iceberg/rest/requests/TestUpdateTableRequestParser.java @@ -23,8 +23,11 @@ import com.fasterxml.jackson.databind.JsonNode; import org.apache.iceberg.MetadataUpdate; +import org.apache.iceberg.Schema; +import org.apache.iceberg.UpdateRequirement; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; +import org.apache.iceberg.types.Types; import org.junit.jupiter.api.Test; public class TestUpdateTableRequestParser { @@ -195,6 +198,24 @@ public void roundTripSerdeWithoutIdentifier() { .isEqualTo(expectedJson); } + @Test + public void serializeAddSchemaWithoutDeprecatedLastColumnId() { + Schema schemaAfterDroppingHighestId = + new Schema(Types.NestedField.required(1, "id", Types.LongType.get())); + UpdateTableRequest request = + UpdateTableRequest.create( + TableIdentifier.of("ns1", "table1"), + ImmutableList.of(new UpdateRequirement.AssertLastAssignedFieldId(2)), + ImmutableList.of(new MetadataUpdate.AddSchema(schemaAfterDroppingHighestId))); + + String json = UpdateTableRequestParser.toJson(request, true); + + assertThat(json).contains("\"action\" : \"add-schema\""); + assertThat(json).contains("\"type\" : \"assert-last-assigned-field-id\""); + assertThat(json).contains("\"last-assigned-field-id\" : 2"); + assertThat(json).doesNotContain("last-column-id"); + } + @Test public void emptyRequirementsAndUpdates() { UpdateTableRequest request =