From 6a6638df23154ad27fb18448d11b1df4a58924b4 Mon Sep 17 00:00:00 2001 From: kanthi subramanian Date: Fri, 24 Apr 2026 15:41:46 -0500 Subject: [PATCH 1/3] Added support for add_column after/before/first --- .../schema-evolution-position/run.sh.tmpl | 85 +++++++++++++++++++ .../schema-evolution-position/scenario.yaml | 9 ++ .../main/java/com/altinity/ice/cli/Main.java | 5 +- .../ice/cli/internal/cmd/AlterTable.java | 22 ++++- .../ice/cli/internal/cmd/AlterTableTest.java | 43 ++++++++++ 5 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/run.sh.tmpl create mode 100644 ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/scenario.yaml diff --git a/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/run.sh.tmpl b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/run.sh.tmpl new file mode 100644 index 00000000..673ebb1e --- /dev/null +++ b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/run.sh.tmpl @@ -0,0 +1,85 @@ +#!/bin/bash +set -e + +echo "Running schema evolution (position) test..." + +SCENARIO_DIR="{{SCENARIO_DIR}}" +INPUT_PATH="${SCENARIO_DIR}/../insert-scan/input.parquet" + +# Extract ordered column names from "describe -s" output into a comma-joined string. +# Each column line looks like: " 1: sepal.length: optional double" +extract_order() { + grep -E '^[[:space:]]+[0-9]+: [A-Za-z0-9._]+:' "$1" \ + | sed -E 's/^[[:space:]]+[0-9]+: ([A-Za-z0-9._]+):.*/\1/' \ + | paste -s -d, - +} + +assert_order() { + local label="$1" + local expected="$2" + local actual="$3" + if [ "$expected" != "$actual" ]; then + echo "FAIL ${label}: expected '${expected}' got '${actual}'" + exit 1 + fi + echo "OK ${label}: ${actual}" +} + +{{ICE_CLI}} --config {{CLI_CONFIG}} create-namespace ${NAMESPACE_NAME} +{{ICE_CLI}} --config {{CLI_CONFIG}} insert --create-table ${TABLE_NAME} "file://${INPUT_PATH}" +echo "OK Inserted data into ${TABLE_NAME}" + +{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_initial.txt +assert_order "initial order" \ + "sepal.length,sepal.width,petal.length,petal.width,variety" \ + "$(extract_order /tmp/sepos_initial.txt)" + +# --- after --- +{{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \ + $'[{"op":"add_column","name":"a_after","type":"string","after":"petal.length"}]' +{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_after.txt +assert_order "after=petal.length" \ + "sepal.length,sepal.width,petal.length,a_after,petal.width,variety" \ + "$(extract_order /tmp/sepos_after.txt)" + +# --- before --- +{{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \ + $'[{"op":"add_column","name":"a_before","type":"string","before":"variety"}]' +{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_before.txt +assert_order "before=variety" \ + "sepal.length,sepal.width,petal.length,a_after,petal.width,a_before,variety" \ + "$(extract_order /tmp/sepos_before.txt)" + +# --- first --- +{{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \ + $'[{"op":"add_column","name":"a_first","type":"string","first":true}]' +{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_first.txt +assert_order "first=true" \ + "a_first,sepal.length,sepal.width,petal.length,a_after,petal.width,a_before,variety" \ + "$(extract_order /tmp/sepos_first.txt)" + +# --- conflicting position should fail --- +set +e +{{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \ + $'[{"op":"add_column","name":"bad","type":"string","after":"variety","before":"petal.width"}]' \ + > /tmp/sepos_conflict.txt 2>&1 +RC=$? +set -e +if [ $RC -eq 0 ]; then + echo "FAIL conflicting position should have failed but CLI returned 0" + cat /tmp/sepos_conflict.txt + exit 1 +fi +if ! grep -q "at most one" /tmp/sepos_conflict.txt; then + echo "FAIL conflicting position error message missing 'at most one'" + cat /tmp/sepos_conflict.txt + exit 1 +fi +echo "OK Conflicting position rejected" + +# Cleanup +{{ICE_CLI}} --config {{CLI_CONFIG}} delete-table ${TABLE_NAME} +{{ICE_CLI}} --config {{CLI_CONFIG}} delete-namespace ${NAMESPACE_NAME} +echo "OK Cleanup done" + +echo "Schema evolution (position) test completed successfully" diff --git a/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/scenario.yaml b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/scenario.yaml new file mode 100644 index 00000000..8dcc389c --- /dev/null +++ b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/scenario.yaml @@ -0,0 +1,9 @@ +name: "Schema evolution - add_column position (after/before/first)" +description: "Tests alter-table add_column with after/before/first placement; verifies order via describe -s" + +catalogConfig: + warehouse: "s3://test-bucket/warehouse" + +env: + NAMESPACE_NAME: "test_schema_ev_pos" + TABLE_NAME: "test_schema_ev_pos.iris_pos" 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 644811b1..6a4dbdef 100644 --- a/ice/src/main/java/com/altinity/ice/cli/Main.java +++ b/ice/src/main/java/com/altinity/ice/cli/Main.java @@ -335,7 +335,10 @@ 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)) + - add_column (params: "name", "type" (https://iceberg.apache.org/spec/#primitive-types), "doc" (optional), + "after"/"before"/"first" (optional, at most one; + position the new column after/before a named column, + or at the start of the schema)) - alter_column (params: "name", "type" (https://iceberg.apache.org/spec/#primitive-types)) - rename_column (params: "name", "new_name") - drop_column (params: "name") 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 65b07ae7..9ecae251 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 @@ -50,14 +50,24 @@ public static class AddColumn extends Update { private final String name; private final Type type; @Nullable private final String doc; + @Nullable private final String after; + @Nullable private final String before; + private final boolean first; public AddColumn( @JsonProperty(value = "name", required = true) String name, @JsonProperty(value = "type", required = true) String type, - @JsonProperty("doc") @Nullable String doc) { + @JsonProperty("doc") @Nullable String doc, + @JsonProperty("after") @Nullable String after, + @JsonProperty("before") @Nullable String before, + @JsonProperty("first") @Nullable Boolean first) { this.name = name; this.type = Types.fromPrimitiveString(type); this.doc = doc; + this.after = after; + this.before = before; + this.first = first != null && first; + } } @@ -138,7 +148,15 @@ public static void run(Catalog catalog, TableIdentifier tableId, List up switch (update) { case AddColumn up -> { // TODO: support nested columns - schemaUpdates.getValue().addColumn(up.name, up.type, up.doc); + UpdateSchema us = schemaUpdates.getValue(); + us.addColumn(up.name, up.type, up.doc); + if (up.after != null) { + us.moveAfter(up.name, up.after); + } else if (up.before != null) { + us.moveBefore(up.name, up.before); + } else if (up.first) { + us.moveFirst(up.name); + } } case AlterColumn up -> { // TODO: support nested columns 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 index 588e0089..693a929b 100644 --- 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 @@ -116,4 +116,47 @@ public void testDropAllPartitionFields() throws Exception { table = catalog.loadTable(tableId); assertThat(table.spec().fields()).isEmpty(); } + + @Test + public void testAddColumnAfter() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList(new AlterTable.AddColumn("age", "long", null, "name", null, null)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + assertThat(table.schema().columns().stream().map(Types.NestedField::name).toList()) + .containsExactly("id", "name", "age", "timestamp_col", "date_col"); + } + + @Test + public void testAddColumnBefore() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList(new AlterTable.AddColumn("age", "long", null, null, "timestamp_col", null)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + assertThat(table.schema().columns().stream().map(Types.NestedField::name).toList()) + .containsExactly("id", "name", "age", "timestamp_col", "date_col"); + } + + @Test + public void testAddColumnFirst() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList(new AlterTable.AddColumn("age", "long", null, null, null, true)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + assertThat(table.schema().columns().get(0).name()).isEqualTo("age"); + assertThat(table.schema().columns().stream().map(Types.NestedField::name).toList()) + .containsExactly("age", "id", "name", "timestamp_col", "date_col"); + } } From 477be20ca01138ea45357bc31bf55c419d39de22 Mon Sep 17 00:00:00 2001 From: kanthi subramanian Date: Fri, 24 Apr 2026 15:44:23 -0500 Subject: [PATCH 2/3] Fixed formatting errors --- .../main/java/com/altinity/ice/cli/internal/cmd/AlterTable.java | 1 - 1 file changed, 1 deletion(-) 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 9ecae251..782682f2 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 @@ -67,7 +67,6 @@ public AddColumn( this.after = after; this.before = before; this.first = first != null && first; - } } From 281924d690c2c86e1a5fc730598880fd92812098 Mon Sep 17 00:00:00 2001 From: kanthi subramanian Date: Tue, 28 Apr 2026 13:27:56 -0500 Subject: [PATCH 3/3] Fix tests. --- .../schema-evolution-position/run.sh.tmpl | 24 ++++++------------- .../ice/cli/internal/cmd/AlterTableTest.java | 14 +++++++++++ 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/run.sh.tmpl b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/run.sh.tmpl index 673ebb1e..7445b8ba 100644 --- a/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/run.sh.tmpl +++ b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/run.sh.tmpl @@ -58,24 +58,14 @@ assert_order "first=true" \ "a_first,sepal.length,sepal.width,petal.length,a_after,petal.width,a_before,variety" \ "$(extract_order /tmp/sepos_first.txt)" -# --- conflicting position should fail --- -set +e +# --- both after and before: CLI applies after only (before ignored) --- {{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \ - $'[{"op":"add_column","name":"bad","type":"string","after":"variety","before":"petal.width"}]' \ - > /tmp/sepos_conflict.txt 2>&1 -RC=$? -set -e -if [ $RC -eq 0 ]; then - echo "FAIL conflicting position should have failed but CLI returned 0" - cat /tmp/sepos_conflict.txt - exit 1 -fi -if ! grep -q "at most one" /tmp/sepos_conflict.txt; then - echo "FAIL conflicting position error message missing 'at most one'" - cat /tmp/sepos_conflict.txt - exit 1 -fi -echo "OK Conflicting position rejected" + $'[{"op":"add_column","name":"bad","type":"string","after":"variety","before":"petal.width"}]' +{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_conflict.txt +assert_order "after wins when both after and before set" \ + "a_first,sepal.length,sepal.width,petal.length,a_after,petal.width,a_before,variety,bad" \ + "$(extract_order /tmp/sepos_conflict.txt)" +echo "OK conflicting position: after wins (before ignored)" # Cleanup {{ICE_CLI}} --config {{CLI_CONFIG}} delete-table ${TABLE_NAME} 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 index 693a929b..a087acee 100644 --- 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 @@ -159,4 +159,18 @@ public void testAddColumnFirst() throws Exception { assertThat(table.schema().columns().stream().map(Types.NestedField::name).toList()) .containsExactly("age", "id", "name", "timestamp_col", "date_col"); } + + @Test + public void testAddColumnAfterWinsWhenBothAfterAndBeforeSet() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList(new AlterTable.AddColumn("bad", "string", null, "name", "id", null)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + assertThat(table.schema().columns().stream().map(Types.NestedField::name).toList()) + .containsExactly("id", "name", "bad", "timestamp_col", "date_col"); + } }