Skip to content

Commit 48345e2

Browse files
subkanthisubkanthi
authored
Added support for add_column after/before/first (#147)
1 parent 6334f70 commit 48345e2

5 files changed

Lines changed: 164 additions & 3 deletions

File tree

‎ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/run.sh.tmpl‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
#!/bin/bash
2+
set -e
3+
4+
echo "Running schema evolution (position) test..."
5+
6+
SCENARIO_DIR="{{SCENARIO_DIR}}"
7+
INPUT_PATH="${SCENARIO_DIR}/../insert-scan/input.parquet"
8+
9+
# Extract ordered column names from "describe -s" output into a comma-joined string.
10+
# Each column line looks like: " 1: sepal.length: optional double"
11+
extract_order() {
12+
grep -E '^[[:space:]]+[0-9]+: [A-Za-z0-9._]+:' "$1" \
13+
| sed -E 's/^[[:space:]]+[0-9]+: ([A-Za-z0-9._]+):.*/\1/' \
14+
| paste -s -d, -
15+
}
16+
17+
assert_order() {
18+
local label="$1"
19+
local expected="$2"
20+
local actual="$3"
21+
if [ "$expected" != "$actual" ]; then
22+
echo "FAIL ${label}: expected '${expected}' got '${actual}'"
23+
exit 1
24+
fi
25+
echo "OK ${label}: ${actual}"
26+
}
27+
28+
{{ICE_CLI}} --config {{CLI_CONFIG}} create-namespace ${NAMESPACE_NAME}
29+
{{ICE_CLI}} --config {{CLI_CONFIG}} insert --create-table ${TABLE_NAME} "file://${INPUT_PATH}"
30+
echo "OK Inserted data into ${TABLE_NAME}"
31+
32+
{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_initial.txt
33+
assert_order "initial order" \
34+
"sepal.length,sepal.width,petal.length,petal.width,variety" \
35+
"$(extract_order /tmp/sepos_initial.txt)"
36+
37+
# --- after ---
38+
{{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \
39+
$'[{"op":"add_column","name":"a_after","type":"string","after":"petal.length"}]'
40+
{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_after.txt
41+
assert_order "after=petal.length" \
42+
"sepal.length,sepal.width,petal.length,a_after,petal.width,variety" \
43+
"$(extract_order /tmp/sepos_after.txt)"
44+
45+
# --- before ---
46+
{{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \
47+
$'[{"op":"add_column","name":"a_before","type":"string","before":"variety"}]'
48+
{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_before.txt
49+
assert_order "before=variety" \
50+
"sepal.length,sepal.width,petal.length,a_after,petal.width,a_before,variety" \
51+
"$(extract_order /tmp/sepos_before.txt)"
52+
53+
# --- first ---
54+
{{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \
55+
$'[{"op":"add_column","name":"a_first","type":"string","first":true}]'
56+
{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_first.txt
57+
assert_order "first=true" \
58+
"a_first,sepal.length,sepal.width,petal.length,a_after,petal.width,a_before,variety" \
59+
"$(extract_order /tmp/sepos_first.txt)"
60+
61+
# --- both after and before: CLI applies after only (before ignored) ---
62+
{{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \
63+
$'[{"op":"add_column","name":"bad","type":"string","after":"variety","before":"petal.width"}]'
64+
{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/sepos_conflict.txt
65+
assert_order "after wins when both after and before set" \
66+
"a_first,sepal.length,sepal.width,petal.length,a_after,petal.width,a_before,variety,bad" \
67+
"$(extract_order /tmp/sepos_conflict.txt)"
68+
echo "OK conflicting position: after wins (before ignored)"
69+
70+
# Cleanup
71+
{{ICE_CLI}} --config {{CLI_CONFIG}} delete-table ${TABLE_NAME}
72+
{{ICE_CLI}} --config {{CLI_CONFIG}} delete-namespace ${NAMESPACE_NAME}
73+
echo "OK Cleanup done"
74+
75+
echo "Schema evolution (position) test completed successfully"

‎ice-rest-catalog/src/test/resources/scenarios/schema-evolution-position/scenario.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
name: "Schema evolution - add_column position (after/before/first)"
2+
description: "Tests alter-table add_column with after/before/first placement; verifies order via describe -s"
3+
4+
catalogConfig:
5+
warehouse: "s3://test-bucket/warehouse"
6+
7+
env:
8+
NAMESPACE_NAME: "test_schema_ev_pos"
9+
TABLE_NAME: "test_schema_ev_pos.iris_pos"

‎ice/src/main/java/com/altinity/ice/cli/Main.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,10 @@ void alterTable(
401401
e.g. [{"op":"drop_column","name":"foo"}]
402402
403403
Supported operations:
404-
- add_column (params: "name", "type" (https://iceberg.apache.org/spec/#primitive-types), "doc" (optional))
404+
- add_column (params: "name", "type" (https://iceberg.apache.org/spec/#primitive-types), "doc" (optional),
405+
"after"/"before"/"first" (optional, at most one;
406+
position the new column after/before a named column,
407+
or at the start of the schema))
405408
- alter_column (params: "name", "type" (https://iceberg.apache.org/spec/#primitive-types))
406409
- rename_column (params: "name", "new_name")
407410
- drop_column (params: "name")

‎ice/src/main/java/com/altinity/ice/cli/internal/cmd/AlterTable.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,23 @@ public static class AddColumn extends Update {
5050
private final String name;
5151
private final Type type;
5252
@Nullable private final String doc;
53+
@Nullable private final String after;
54+
@Nullable private final String before;
55+
private final boolean first;
5356

5457
public AddColumn(
5558
@JsonProperty(value = "name", required = true) String name,
5659
@JsonProperty(value = "type", required = true) String type,
57-
@JsonProperty("doc") @Nullable String doc) {
60+
@JsonProperty("doc") @Nullable String doc,
61+
@JsonProperty("after") @Nullable String after,
62+
@JsonProperty("before") @Nullable String before,
63+
@JsonProperty("first") @Nullable Boolean first) {
5864
this.name = name;
5965
this.type = Types.fromPrimitiveString(type);
6066
this.doc = doc;
67+
this.after = after;
68+
this.before = before;
69+
this.first = first != null && first;
6170
}
6271
}
6372

@@ -138,7 +147,15 @@ public static void run(Catalog catalog, TableIdentifier tableId, List<Update> up
138147
switch (update) {
139148
case AddColumn up -> {
140149
// TODO: support nested columns
141-
schemaUpdates.getValue().addColumn(up.name, up.type, up.doc);
150+
UpdateSchema us = schemaUpdates.getValue();
151+
us.addColumn(up.name, up.type, up.doc);
152+
if (up.after != null) {
153+
us.moveAfter(up.name, up.after);
154+
} else if (up.before != null) {
155+
us.moveBefore(up.name, up.before);
156+
} else if (up.first) {
157+
us.moveFirst(up.name);
158+
}
142159
}
143160
case AlterColumn up -> {
144161
// TODO: support nested columns

‎ice/src/test/java/com/altinity/ice/cli/internal/cmd/AlterTableTest.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,61 @@ public void testDropAllPartitionFields() throws Exception {
116116
table = catalog.loadTable(tableId);
117117
assertThat(table.spec().fields()).isEmpty();
118118
}
119+
120+
@Test
121+
public void testAddColumnAfter() throws Exception {
122+
catalog.buildTable(tableId, schema).create();
123+
124+
List<AlterTable.Update> updates =
125+
Arrays.asList(new AlterTable.AddColumn("age", "long", null, "name", null, null));
126+
127+
AlterTable.run(catalog, tableId, updates);
128+
129+
Table table = catalog.loadTable(tableId);
130+
assertThat(table.schema().columns().stream().map(Types.NestedField::name).toList())
131+
.containsExactly("id", "name", "age", "timestamp_col", "date_col");
132+
}
133+
134+
@Test
135+
public void testAddColumnBefore() throws Exception {
136+
catalog.buildTable(tableId, schema).create();
137+
138+
List<AlterTable.Update> updates =
139+
Arrays.asList(new AlterTable.AddColumn("age", "long", null, null, "timestamp_col", null));
140+
141+
AlterTable.run(catalog, tableId, updates);
142+
143+
Table table = catalog.loadTable(tableId);
144+
assertThat(table.schema().columns().stream().map(Types.NestedField::name).toList())
145+
.containsExactly("id", "name", "age", "timestamp_col", "date_col");
146+
}
147+
148+
@Test
149+
public void testAddColumnFirst() throws Exception {
150+
catalog.buildTable(tableId, schema).create();
151+
152+
List<AlterTable.Update> updates =
153+
Arrays.asList(new AlterTable.AddColumn("age", "long", null, null, null, true));
154+
155+
AlterTable.run(catalog, tableId, updates);
156+
157+
Table table = catalog.loadTable(tableId);
158+
assertThat(table.schema().columns().get(0).name()).isEqualTo("age");
159+
assertThat(table.schema().columns().stream().map(Types.NestedField::name).toList())
160+
.containsExactly("age", "id", "name", "timestamp_col", "date_col");
161+
}
162+
163+
@Test
164+
public void testAddColumnAfterWinsWhenBothAfterAndBeforeSet() throws Exception {
165+
catalog.buildTable(tableId, schema).create();
166+
167+
List<AlterTable.Update> updates =
168+
Arrays.asList(new AlterTable.AddColumn("bad", "string", null, "name", "id", null));
169+
170+
AlterTable.run(catalog, tableId, updates);
171+
172+
Table table = catalog.loadTable(tableId);
173+
assertThat(table.schema().columns().stream().map(Types.NestedField::name).toList())
174+
.containsExactly("id", "name", "bad", "timestamp_col", "date_col");
175+
}
119176
}

0 commit comments

Comments
 (0)