From 09874268d18c8c362caa17984fc5b1706b6af582 Mon Sep 17 00:00:00 2001 From: Kanthi Subramanian Date: Fri, 10 Oct 2025 14:01:23 -0500 Subject: [PATCH 1/2] Added drop partition field in alter table. --- .../main/java/com/altinity/ice/cli/Main.java | 13 +++++++------ .../ice/cli/internal/cmd/AlterTable.java | 17 +++++++++++++++++ .../ice/cli/internal/iceberg/Sorting.java | 7 ++----- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/ice/src/main/java/com/altinity/ice/cli/Main.java b/ice/src/main/java/com/altinity/ice/cli/Main.java index 3e22973..d299eb3 100644 --- a/ice/src/main/java/com/altinity/ice/cli/Main.java +++ b/ice/src/main/java/com/altinity/ice/cli/Main.java @@ -225,12 +225,13 @@ void alterTable( e.g. [{"op":"drop_column","name":"foo"}] Supported operations: - - add_column (params: "name", "type" (https://iceberg.apache.org/spec/#primitive-types), "doc" (optional)) - - alter_column (params: "name", "type" (https://iceberg.apache.org/spec/#primitive-types)) - - rename_column (params: "name", "new_name") - - drop_column (params: "name") - - set_tblproperty (params: "key", "value" (set to null to remove table property)) - - rename_to (params: "new_name") + - add_column (params: "name", "type" (https://iceberg.apache.org/spec/#primitive-types), "doc" (optional)) + - alter_column (params: "name", "type" (https://iceberg.apache.org/spec/#primitive-types)) + - rename_column (params: "name", "new_name") + - drop_column (params: "name") + - set_tblproperty (params: "key", "value" (set to null to remove table property)) + - rename_to (params: "new_name") + - drop_partition_field (params: "name") """) String updatesJson) throws IOException { diff --git a/ice/src/main/java/com/altinity/ice/cli/internal/cmd/AlterTable.java b/ice/src/main/java/com/altinity/ice/cli/internal/cmd/AlterTable.java index e8a5421..65b07ae 100644 --- a/ice/src/main/java/com/altinity/ice/cli/internal/cmd/AlterTable.java +++ b/ice/src/main/java/com/altinity/ice/cli/internal/cmd/AlterTable.java @@ -17,6 +17,7 @@ import javax.annotation.Nullable; import org.apache.iceberg.Table; import org.apache.iceberg.Transaction; +import org.apache.iceberg.UpdatePartitionSpec; import org.apache.iceberg.UpdateProperties; import org.apache.iceberg.UpdateSchema; import org.apache.iceberg.catalog.Catalog; @@ -41,6 +42,7 @@ private AlterTable() {} @JsonSubTypes.Type(value = DropColumn.class, name = "drop_column"), @JsonSubTypes.Type(value = SetTblProperty.class, name = "set_tblproperty"), @JsonSubTypes.Type(value = RenameTo.class, name = "rename_to"), + @JsonSubTypes.Type(value = DropPartitionField.class, name = "drop_partition_field"), }) public abstract static class Update {} @@ -111,6 +113,14 @@ public RenameTo(@JsonProperty(value = "new_name", required = true) String newNam } } + public static class DropPartitionField extends Update { + private final String name; + + public DropPartitionField(@JsonProperty(value = "name", required = true) String name) { + this.name = name; + } + } + public static void run(Catalog catalog, TableIdentifier tableId, List updates) throws IOException { if (updates.isEmpty()) { @@ -122,6 +132,7 @@ public static void run(Catalog catalog, TableIdentifier tableId, List up Transaction tx = table.newTransaction(); Lazy schemaUpdates = new Lazy<>(tx::updateSchema); Lazy propertiesUpdates = new Lazy<>(tx::updateProperties); + Lazy partitionSpecUpdates = new Lazy<>(tx::updateSpec); RenameTo renameTo = null; for (Update update : updates) { switch (update) { @@ -150,6 +161,9 @@ public static void run(Catalog catalog, TableIdentifier tableId, List up case RenameTo up -> { renameTo = up; } + case DropPartitionField up -> { + partitionSpecUpdates.getValue().removeField(up.name); + } default -> throw new UnsupportedOperationException(); } } @@ -159,6 +173,9 @@ public static void run(Catalog catalog, TableIdentifier tableId, List up if (propertiesUpdates.hasValue()) { propertiesUpdates.getValue().commit(); } + if (partitionSpecUpdates.hasValue()) { + partitionSpecUpdates.getValue().commit(); + } tx.commitTransaction(); if (renameTo != null) { catalog.renameTable(tableId, TableIdentifier.parse(renameTo.newName)); diff --git a/ice/src/main/java/com/altinity/ice/cli/internal/iceberg/Sorting.java b/ice/src/main/java/com/altinity/ice/cli/internal/iceberg/Sorting.java index c972752..63730f1 100644 --- a/ice/src/main/java/com/altinity/ice/cli/internal/iceberg/Sorting.java +++ b/ice/src/main/java/com/altinity/ice/cli/internal/iceberg/Sorting.java @@ -25,7 +25,6 @@ import org.apache.iceberg.io.CloseableIterable; import org.apache.iceberg.io.CloseableIterator; import org.apache.iceberg.io.InputFile; -import org.apache.iceberg.mapping.NameMapping; import org.apache.iceberg.parquet.Parquet; import org.apache.iceberg.types.Types; @@ -100,16 +99,14 @@ public String toUnsortedDiffString() { } } - public static boolean isSorted( - InputFile inputFile, Schema tableSchema, SortOrder sortOrder) + public static boolean isSorted(InputFile inputFile, Schema tableSchema, SortOrder sortOrder) throws IOException { return checkSorted(inputFile, tableSchema, sortOrder).ok; } // TODO: check metadata first to avoid full scan when unsorted public static SortCheckResult checkSorted( - InputFile inputFile, Schema tableSchema, SortOrder sortOrder) - throws IOException { + InputFile inputFile, Schema tableSchema, SortOrder sortOrder) throws IOException { if (sortOrder.isUnsorted()) { return new SortCheckResult(false); } From 7d2ec5e7caedbf871eca6edaa4fd5000e5ce4dd9 Mon Sep 17 00:00:00 2001 From: Kanthi Subramanian Date: Fri, 10 Oct 2025 14:05:34 -0500 Subject: [PATCH 2/2] Added AlterTableTest using InMemoryCatalog. --- .../ice/cli/internal/cmd/AlterTableTest.java | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 ice/src/test/java/com/altinity/ice/cli/internal/cmd/AlterTableTest.java diff --git a/ice/src/test/java/com/altinity/ice/cli/internal/cmd/AlterTableTest.java b/ice/src/test/java/com/altinity/ice/cli/internal/cmd/AlterTableTest.java new file mode 100644 index 0000000..588e008 --- /dev/null +++ b/ice/src/test/java/com/altinity/ice/cli/internal/cmd/AlterTableTest.java @@ -0,0 +1,119 @@ +/* + * Copyright (c) 2025 Altinity Inc and/or its affiliates. All rights reserved. + * + * Licensed 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 + */ +package com.altinity.ice.cli.internal.cmd; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.Arrays; +import java.util.List; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.types.Types; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +public class AlterTableTest { + + private InMemoryCatalog catalog; + private TableIdentifier tableId; + private Schema schema; + + @BeforeMethod + public void setUp() { + catalog = new InMemoryCatalog(); + catalog.initialize("test-catalog", java.util.Map.of()); + tableId = TableIdentifier.of("test", "table1"); + + // create namespace. + catalog.createNamespace(org.apache.iceberg.catalog.Namespace.of("test")); + schema = + new Schema( + Types.NestedField.required(1, "id", Types.LongType.get()), + Types.NestedField.required(2, "name", Types.StringType.get()), + Types.NestedField.required(3, "timestamp_col", Types.TimestampType.withZone()), + Types.NestedField.required(4, "date_col", Types.DateType.get())); + } + + @Test + public void testDropPartitionField() throws Exception { + PartitionSpec partitionSpec = + PartitionSpec.builderFor(schema).identity("name").year("timestamp_col").build(); + + Table table = catalog.buildTable(tableId, schema).withPartitionSpec(partitionSpec).create(); + + assertThat(table.spec().fields()).hasSize(2); + assertThat(table.spec().fields().get(0).name()).isEqualTo("name"); + assertThat(table.spec().fields().get(1).name()).isEqualTo("timestamp_col_year"); + + List updates = Arrays.asList(new AlterTable.DropPartitionField("name")); + + AlterTable.run(catalog, tableId, updates); + + table = catalog.loadTable(tableId); + assertThat(table.spec().fields()).hasSize(1); + assertThat(table.spec().fields().get(0).name()).isEqualTo("timestamp_col_year"); + } + + @Test + public void testDropPartitionFieldByTransformName() throws Exception { + PartitionSpec partitionSpec = + PartitionSpec.builderFor(schema).identity("name").year("timestamp_col").build(); + + Table table = catalog.buildTable(tableId, schema).withPartitionSpec(partitionSpec).create(); + + assertThat(table.spec().fields()).hasSize(2); + + List updates = + Arrays.asList(new AlterTable.DropPartitionField("timestamp_col_year")); + + AlterTable.run(catalog, tableId, updates); + + table = catalog.loadTable(tableId); + assertThat(table.spec().fields()).hasSize(1); + assertThat(table.spec().fields().get(0).name()).isEqualTo("name"); + } + + @Test + public void testDropNonExistentPartitionField() throws Exception { + PartitionSpec partitionSpec = PartitionSpec.builderFor(schema).identity("name").build(); + + catalog.buildTable(tableId, schema).withPartitionSpec(partitionSpec).create(); + + List updates = + Arrays.asList(new AlterTable.DropPartitionField("non_existent_field")); + + assertThatThrownBy(() -> AlterTable.run(catalog, tableId, updates)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + public void testDropAllPartitionFields() throws Exception { + PartitionSpec partitionSpec = + PartitionSpec.builderFor(schema).identity("name").year("timestamp_col").build(); + + Table table = catalog.buildTable(tableId, schema).withPartitionSpec(partitionSpec).create(); + + assertThat(table.spec().fields()).hasSize(2); + + List updates = + Arrays.asList( + new AlterTable.DropPartitionField("name"), + new AlterTable.DropPartitionField("timestamp_col_year")); + + AlterTable.run(catalog, tableId, updates); + + table = catalog.loadTable(tableId); + assertThat(table.spec().fields()).isEmpty(); + } +}