From b1f9c286aeebb353c565931f8d821dceb1943ce1 Mon Sep 17 00:00:00 2001 From: kanthi subramanian Date: Mon, 29 Jun 2026 09:52:43 -0400 Subject: [PATCH 1/4] Added logic to add non-primitive types to alter table add column. --- ice/README.md | 6 + .../main/java/com/altinity/ice/cli/Main.java | 7 +- .../ice/cli/internal/cmd/AlterTable.java | 3 +- .../cli/internal/util/IcebergTypeParser.java | 157 ++++++++++++++++ .../ice/cli/internal/cmd/AlterTableTest.java | 145 +++++++++++++++ .../internal/util/IcebergTypeParserTest.java | 172 ++++++++++++++++++ 6 files changed, 487 insertions(+), 3 deletions(-) create mode 100644 ice/src/main/java/com/altinity/ice/cli/internal/util/IcebergTypeParser.java create mode 100644 ice/src/test/java/com/altinity/ice/cli/internal/util/IcebergTypeParserTest.java diff --git a/ice/README.md b/ice/README.md index 890a920e..c5d5f5b9 100644 --- a/ice/README.md +++ b/ice/README.md @@ -102,6 +102,12 @@ ice alter-table flowers.iris '[{"op":"add_column","name":"extra","type":"string" # add a NOT NULL column (`required: true`;) ice alter-table flowers.iris '[{"op":"add_column","name":"extra","type":"string","required":true}]' +# add a list column +ice alter-table flowers.iris '[{"op":"add_column","name":"tags","type":"list"}]' + +# add a map column +ice alter-table flowers.iris '[{"op":"add_column","name":"metadata","type":"map"}]' + # verify the schema change ice describe -s flowers.iris ``` 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 e0a85cc0..7ea5257b 100644 --- a/ice/src/main/java/com/altinity/ice/cli/Main.java +++ b/ice/src/main/java/com/altinity/ice/cli/Main.java @@ -405,10 +405,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), + - add_column (params: "name", "type", "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)) + or at the start of the schema). + "type" supports primitives (https://iceberg.apache.org/spec/#primitive-types) + and complex types: list, map, struct, + e.g. "list", "map", "struct") - 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 a791877a..3eeb9ce9 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 @@ -9,6 +9,7 @@ */ package com.altinity.ice.cli.internal.cmd; +import com.altinity.ice.cli.internal.util.IcebergTypeParser; import com.altinity.ice.cli.internal.util.UserInputParser; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSubTypes; @@ -70,7 +71,7 @@ public AddColumn( @JsonProperty("required") @Nullable Boolean required, @JsonProperty("initial_default") @Nullable String initialDefault) { this.name = name; - this.type = Types.fromPrimitiveString(type); + this.type = IcebergTypeParser.parseType(type); this.doc = doc; this.after = after; this.before = before; diff --git a/ice/src/main/java/com/altinity/ice/cli/internal/util/IcebergTypeParser.java b/ice/src/main/java/com/altinity/ice/cli/internal/util/IcebergTypeParser.java new file mode 100644 index 00000000..488d1921 --- /dev/null +++ b/ice/src/main/java/com/altinity/ice/cli/internal/util/IcebergTypeParser.java @@ -0,0 +1,157 @@ +/* + * 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.util; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.iceberg.types.Type; +import org.apache.iceberg.types.Types; + +/** + * Parses type strings including complex types ({@code list}, {@code map}, + * {@code struct}) into Iceberg {@link Type} instances. + * + *

Placeholder field IDs are assigned during parsing. Iceberg's {@code UpdateSchema.addColumn} + * reassigns fresh IDs from the table's {@code lastColumnId} on commit, so placeholder values are + * safe. + */ +public final class IcebergTypeParser { + + private IcebergTypeParser() {} + + public static Type parseType(String typeString) { + return parseType(typeString, new AtomicInteger(0)); + } + + private static Type parseType(String typeString, AtomicInteger nextId) { + String s = typeString.strip(); + if (s.isEmpty()) { + throw new IllegalArgumentException("Type string must not be empty"); + } + + String lower = s.toLowerCase(); + + if (lower.startsWith("list<")) { + return parseList(s, nextId); + } + if (lower.startsWith("map<")) { + return parseMap(s, nextId); + } + if (lower.startsWith("struct<")) { + return parseStruct(s, nextId); + } + + return Types.fromPrimitiveString(s); + } + + private static Types.ListType parseList(String s, AtomicInteger nextId) { + String inner = unwrapAngleBrackets(s, "list"); + int elementId = nextId.incrementAndGet(); + Type elementType = parseType(inner, nextId); + return Types.ListType.ofOptional(elementId, elementType); + } + + private static Types.MapType parseMap(String s, AtomicInteger nextId) { + String inner = unwrapAngleBrackets(s, "map"); + List parts = splitAtTopLevelComma(inner); + if (parts.size() != 2) { + throw new IllegalArgumentException( + "map type requires exactly 2 type parameters (key and value), got: " + s); + } + int keyId = nextId.incrementAndGet(); + int valueId = nextId.incrementAndGet(); + Type keyType = parseType(parts.get(0), nextId); + Type valueType = parseType(parts.get(1), nextId); + return Types.MapType.ofOptional(keyId, valueId, keyType, valueType); + } + + private static Types.StructType parseStruct(String s, AtomicInteger nextId) { + String inner = unwrapAngleBrackets(s, "struct"); + List fieldDefs = splitAtTopLevelComma(inner); + List fields = new ArrayList<>(); + for (String fieldDef : fieldDefs) { + String trimmed = fieldDef.strip(); + int colonIdx = findFieldNameSeparator(trimmed); + if (colonIdx < 0) { + throw new IllegalArgumentException( + "struct field must be in the form 'name:type', got: " + trimmed); + } + String fieldName = trimmed.substring(0, colonIdx).strip(); + String fieldTypeStr = trimmed.substring(colonIdx + 1).strip(); + if (fieldName.isEmpty()) { + throw new IllegalArgumentException("struct field name must not be empty in: " + trimmed); + } + if (fieldTypeStr.isEmpty()) { + throw new IllegalArgumentException("struct field type must not be empty in: " + trimmed); + } + int fieldId = nextId.incrementAndGet(); + Type fieldType = parseType(fieldTypeStr, nextId); + fields.add(Types.NestedField.optional(fieldId, fieldName, fieldType)); + } + return Types.StructType.of(fields); + } + + /** Finds the first colon that separates name from type, skipping colons inside nested types. */ + private static int findFieldNameSeparator(String s) { + int depth = 0; + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + if (c == '<' || c == '(') { + depth++; + } else if (c == '>' || c == ')') { + depth--; + } else if (c == ':' && depth == 0) { + return i; + } + } + return -1; + } + + /** + * Strips the prefix and the surrounding angle brackets, e.g. "list" -> "string". + * Validates that brackets are present and balanced. + */ + private static String unwrapAngleBrackets(String s, String prefix) { + String trimmed = s.strip(); + int prefixLen = prefix.length(); + if (trimmed.length() < prefixLen + 2 + || trimmed.charAt(prefixLen) != '<' + || trimmed.charAt(trimmed.length() - 1) != '>') { + throw new IllegalArgumentException( + "Expected " + prefix + "<...> syntax, got: " + trimmed); + } + String inner = trimmed.substring(prefixLen + 1, trimmed.length() - 1).strip(); + if (inner.isEmpty()) { + throw new IllegalArgumentException(prefix + "<> must not be empty"); + } + return inner; + } + + /** Splits on commas that are not nested inside {@code <...>} or {@code (...)}. */ + private static List splitAtTopLevelComma(String s) { + List parts = new ArrayList<>(); + int depth = 0; + int start = 0; + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + if (c == '<' || c == '(') { + depth++; + } else if (c == '>' || c == ')') { + depth--; + } else if (c == ',' && depth == 0) { + parts.add(s.substring(start, i)); + start = i + 1; + } + } + parts.add(s.substring(start)); + return parts; + } +} 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 ba9931e1..1898305d 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 @@ -19,6 +19,7 @@ import org.apache.iceberg.Table; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.types.Type; import org.apache.iceberg.types.Types; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -202,4 +203,148 @@ public void testAddOptionalColumnByDefault() throws Exception { Table table = catalog.loadTable(tableId); assertThat(table.schema().findField("age").isOptional()).isTrue(); } + + @Test + public void testAddListOfStringColumn() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList( + new AlterTable.AddColumn( + "tags", "list", null, null, null, null, null, null)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + Types.NestedField field = table.schema().findField("tags"); + assertThat(field).isNotNull(); + assertThat(field.isOptional()).isTrue(); + assertThat(field.type()).isInstanceOf(Types.ListType.class); + Types.ListType listType = (Types.ListType) field.type(); + assertThat(listType.elementType()).isInstanceOf(Types.StringType.class); + } + + @Test + public void testAddListOfLongColumn() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList( + new AlterTable.AddColumn( + "scores", "list", null, null, null, null, null, null)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + Types.NestedField field = table.schema().findField("scores"); + assertThat(field).isNotNull(); + assertThat(field.type()).isInstanceOf(Types.ListType.class); + Types.ListType listType = (Types.ListType) field.type(); + assertThat(listType.elementType()).isInstanceOf(Types.LongType.class); + } + + @Test + public void testAddListColumnAfter() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList( + new AlterTable.AddColumn( + "tags", "list", null, "name", null, null, 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", "tags", "timestamp_col", "date_col"); + assertThat(table.schema().findField("tags").type()).isInstanceOf(Types.ListType.class); + } + + @Test + public void testAddMapColumn() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList( + new AlterTable.AddColumn( + "metadata", "map", null, null, null, null, null, null)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + Types.NestedField field = table.schema().findField("metadata"); + assertThat(field).isNotNull(); + assertThat(field.type()).isInstanceOf(Types.MapType.class); + Types.MapType mapType = (Types.MapType) field.type(); + assertThat(mapType.keyType()).isInstanceOf(Types.StringType.class); + assertThat(mapType.valueType()).isInstanceOf(Types.LongType.class); + } + + @Test + public void testAddStructColumn() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList( + new AlterTable.AddColumn( + "address", "struct", null, null, null, null, null, null)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + Types.NestedField field = table.schema().findField("address"); + assertThat(field).isNotNull(); + assertThat(field.type()).isInstanceOf(Types.StructType.class); + Types.StructType structType = (Types.StructType) field.type(); + assertThat(structType.fields()).hasSize(2); + assertThat(structType.field("street").type()).isInstanceOf(Types.StringType.class); + assertThat(structType.field("zip").type()).isInstanceOf(Types.IntegerType.class); + } + + @Test + public void testAddNestedListOfStructColumn() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList( + new AlterTable.AddColumn( + "items", + "list>", + null, + null, + null, + null, + null, + null)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + Types.NestedField field = table.schema().findField("items"); + assertThat(field).isNotNull(); + assertThat(field.type().typeId()).isEqualTo(Type.TypeID.LIST); + Types.ListType listType = (Types.ListType) field.type(); + assertThat(listType.elementType().typeId()).isEqualTo(Type.TypeID.STRUCT); + Types.StructType inner = (Types.StructType) listType.elementType(); + assertThat(inner.fields()).hasSize(2); + assertThat(inner.field("product_id").type()).isInstanceOf(Types.LongType.class); + assertThat(inner.field("name").type()).isInstanceOf(Types.StringType.class); + } + + @Test + public void testPrimitiveTypeStillWorks() throws Exception { + catalog.buildTable(tableId, schema).create(); + + List updates = + Arrays.asList( + new AlterTable.AddColumn( + "score", "double", null, null, null, null, null, null)); + + AlterTable.run(catalog, tableId, updates); + + Table table = catalog.loadTable(tableId); + Types.NestedField field = table.schema().findField("score"); + assertThat(field).isNotNull(); + assertThat(field.type()).isInstanceOf(Types.DoubleType.class); + } } diff --git a/ice/src/test/java/com/altinity/ice/cli/internal/util/IcebergTypeParserTest.java b/ice/src/test/java/com/altinity/ice/cli/internal/util/IcebergTypeParserTest.java new file mode 100644 index 00000000..1e76d80d --- /dev/null +++ b/ice/src/test/java/com/altinity/ice/cli/internal/util/IcebergTypeParserTest.java @@ -0,0 +1,172 @@ +/* + * 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.util; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.apache.iceberg.types.Type; +import org.apache.iceberg.types.Types; +import org.testng.annotations.Test; + +public class IcebergTypeParserTest { + + @Test + public void testPrimitiveString() { + Type type = IcebergTypeParser.parseType("string"); + assertThat(type).isInstanceOf(Types.StringType.class); + } + + @Test + public void testPrimitiveLong() { + Type type = IcebergTypeParser.parseType("long"); + assertThat(type).isInstanceOf(Types.LongType.class); + } + + @Test + public void testPrimitiveInt() { + Type type = IcebergTypeParser.parseType("int"); + assertThat(type).isInstanceOf(Types.IntegerType.class); + } + + @Test + public void testPrimitiveDecimal() { + Type type = IcebergTypeParser.parseType("decimal(10,2)"); + assertThat(type).isInstanceOf(Types.DecimalType.class); + Types.DecimalType decimal = (Types.DecimalType) type; + assertThat(decimal.precision()).isEqualTo(10); + assertThat(decimal.scale()).isEqualTo(2); + } + + @Test + public void testListOfString() { + Type type = IcebergTypeParser.parseType("list"); + assertThat(type).isInstanceOf(Types.ListType.class); + Types.ListType listType = (Types.ListType) type; + assertThat(listType.elementType()).isInstanceOf(Types.StringType.class); + assertThat(listType.isElementOptional()).isTrue(); + } + + @Test + public void testListOfLong() { + Type type = IcebergTypeParser.parseType("list"); + assertThat(type).isInstanceOf(Types.ListType.class); + Types.ListType listType = (Types.ListType) type; + assertThat(listType.elementType()).isInstanceOf(Types.LongType.class); + } + + @Test + public void testMapStringLong() { + Type type = IcebergTypeParser.parseType("map"); + assertThat(type).isInstanceOf(Types.MapType.class); + Types.MapType mapType = (Types.MapType) type; + assertThat(mapType.keyType()).isInstanceOf(Types.StringType.class); + assertThat(mapType.valueType()).isInstanceOf(Types.LongType.class); + } + + @Test + public void testMapWithSpaces() { + Type type = IcebergTypeParser.parseType("map< string , long >"); + assertThat(type).isInstanceOf(Types.MapType.class); + Types.MapType mapType = (Types.MapType) type; + assertThat(mapType.keyType()).isInstanceOf(Types.StringType.class); + assertThat(mapType.valueType()).isInstanceOf(Types.LongType.class); + } + + @Test + public void testSimpleStruct() { + Type type = IcebergTypeParser.parseType("struct"); + assertThat(type).isInstanceOf(Types.StructType.class); + Types.StructType structType = (Types.StructType) type; + assertThat(structType.fields()).hasSize(2); + assertThat(structType.field("a").type()).isInstanceOf(Types.StringType.class); + assertThat(structType.field("b").type()).isInstanceOf(Types.LongType.class); + } + + @Test + public void testNestedListOfStruct() { + Type type = IcebergTypeParser.parseType("list>"); + assertThat(type).isInstanceOf(Types.ListType.class); + Types.ListType listType = (Types.ListType) type; + assertThat(listType.elementType()).isInstanceOf(Types.StructType.class); + Types.StructType inner = (Types.StructType) listType.elementType(); + assertThat(inner.fields()).hasSize(2); + assertThat(inner.field("id").type()).isInstanceOf(Types.LongType.class); + assertThat(inner.field("name").type()).isInstanceOf(Types.StringType.class); + } + + @Test + public void testMapWithComplexValue() { + Type type = IcebergTypeParser.parseType("map>"); + assertThat(type).isInstanceOf(Types.MapType.class); + Types.MapType mapType = (Types.MapType) type; + assertThat(mapType.keyType()).isInstanceOf(Types.StringType.class); + assertThat(mapType.valueType()).isInstanceOf(Types.ListType.class); + Types.ListType valueList = (Types.ListType) mapType.valueType(); + assertThat(valueList.elementType()).isInstanceOf(Types.IntegerType.class); + } + + @Test + public void testStructWithNestedList() { + Type type = IcebergTypeParser.parseType("struct>"); + assertThat(type).isInstanceOf(Types.StructType.class); + Types.StructType structType = (Types.StructType) type; + assertThat(structType.field("name").type()).isInstanceOf(Types.StringType.class); + assertThat(structType.field("tags").type()).isInstanceOf(Types.ListType.class); + } + + @Test + public void testCaseInsensitive() { + Type type = IcebergTypeParser.parseType("LIST"); + assertThat(type).isInstanceOf(Types.ListType.class); + Types.ListType listType = (Types.ListType) type; + assertThat(listType.elementType()).isInstanceOf(Types.StringType.class); + } + + @Test + public void testWhitespaceHandling() { + Type type = IcebergTypeParser.parseType(" list< string > "); + assertThat(type).isInstanceOf(Types.ListType.class); + } + + @Test + public void testEmptyTypeStringThrows() { + assertThatThrownBy(() -> IcebergTypeParser.parseType("")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must not be empty"); + } + + @Test + public void testEmptyListThrows() { + assertThatThrownBy(() -> IcebergTypeParser.parseType("list<>")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must not be empty"); + } + + @Test + public void testMapWrongArityThrows() { + assertThatThrownBy(() -> IcebergTypeParser.parseType("map")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("exactly 2 type parameters"); + } + + @Test + public void testStructMissingColonThrows() { + assertThatThrownBy(() -> IcebergTypeParser.parseType("struct")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("name:type"); + } + + @Test + public void testInvalidPrimitiveThrows() { + assertThatThrownBy(() -> IcebergTypeParser.parseType("foobar")) + .isInstanceOf(IllegalArgumentException.class); + } +} From 74eca4ac493aa0ca507a89fa4f3a95403a952a34 Mon Sep 17 00:00:00 2001 From: kanthi subramanian Date: Mon, 29 Jun 2026 09:58:38 -0400 Subject: [PATCH 2/4] Fixed formatting errors --- .../cli/internal/util/IcebergTypeParser.java | 3 +- .../ice/cli/internal/cmd/AlterTableTest.java | 12 +- .../internal/util/IcebergTypeParserTest.java | 192 +++++------------- 3 files changed, 61 insertions(+), 146 deletions(-) diff --git a/ice/src/main/java/com/altinity/ice/cli/internal/util/IcebergTypeParser.java b/ice/src/main/java/com/altinity/ice/cli/internal/util/IcebergTypeParser.java index 488d1921..8b023eec 100644 --- a/ice/src/main/java/com/altinity/ice/cli/internal/util/IcebergTypeParser.java +++ b/ice/src/main/java/com/altinity/ice/cli/internal/util/IcebergTypeParser.java @@ -125,8 +125,7 @@ private static String unwrapAngleBrackets(String s, String prefix) { if (trimmed.length() < prefixLen + 2 || trimmed.charAt(prefixLen) != '<' || trimmed.charAt(trimmed.length() - 1) != '>') { - throw new IllegalArgumentException( - "Expected " + prefix + "<...> syntax, got: " + trimmed); + throw new IllegalArgumentException("Expected " + prefix + "<...> syntax, got: " + trimmed); } String inner = trimmed.substring(prefixLen + 1, trimmed.length() - 1).strip(); if (inner.isEmpty()) { 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 1898305d..c2a0235b 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 @@ -210,8 +210,7 @@ public void testAddListOfStringColumn() throws Exception { List updates = Arrays.asList( - new AlterTable.AddColumn( - "tags", "list", null, null, null, null, null, null)); + new AlterTable.AddColumn("tags", "list", null, null, null, null, null, null)); AlterTable.run(catalog, tableId, updates); @@ -230,8 +229,7 @@ public void testAddListOfLongColumn() throws Exception { List updates = Arrays.asList( - new AlterTable.AddColumn( - "scores", "list", null, null, null, null, null, null)); + new AlterTable.AddColumn("scores", "list", null, null, null, null, null, null)); AlterTable.run(catalog, tableId, updates); @@ -249,8 +247,7 @@ public void testAddListColumnAfter() throws Exception { List updates = Arrays.asList( - new AlterTable.AddColumn( - "tags", "list", null, "name", null, null, null, null)); + new AlterTable.AddColumn("tags", "list", null, "name", null, null, null, null)); AlterTable.run(catalog, tableId, updates); @@ -337,8 +334,7 @@ public void testPrimitiveTypeStillWorks() throws Exception { List updates = Arrays.asList( - new AlterTable.AddColumn( - "score", "double", null, null, null, null, null, null)); + new AlterTable.AddColumn("score", "double", null, null, null, null, null, null)); AlterTable.run(catalog, tableId, updates); diff --git a/ice/src/test/java/com/altinity/ice/cli/internal/util/IcebergTypeParserTest.java b/ice/src/test/java/com/altinity/ice/cli/internal/util/IcebergTypeParserTest.java index 1e76d80d..ac355b29 100644 --- a/ice/src/test/java/com/altinity/ice/cli/internal/util/IcebergTypeParserTest.java +++ b/ice/src/test/java/com/altinity/ice/cli/internal/util/IcebergTypeParserTest.java @@ -14,159 +14,79 @@ import org.apache.iceberg.types.Type; import org.apache.iceberg.types.Types; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; public class IcebergTypeParserTest { - @Test - public void testPrimitiveString() { - Type type = IcebergTypeParser.parseType("string"); - assertThat(type).isInstanceOf(Types.StringType.class); - } - - @Test - public void testPrimitiveLong() { - Type type = IcebergTypeParser.parseType("long"); - assertThat(type).isInstanceOf(Types.LongType.class); - } - - @Test - public void testPrimitiveInt() { - Type type = IcebergTypeParser.parseType("int"); - assertThat(type).isInstanceOf(Types.IntegerType.class); + @DataProvider(name = "validTypes") + public Object[][] validTypes() { + return new Object[][] { + {"string", Type.TypeID.STRING}, + {"long", Type.TypeID.LONG}, + {"int", Type.TypeID.INTEGER}, + {"boolean", Type.TypeID.BOOLEAN}, + {"float", Type.TypeID.FLOAT}, + {"double", Type.TypeID.DOUBLE}, + {"date", Type.TypeID.DATE}, + {"decimal(10,2)", Type.TypeID.DECIMAL}, + {"list", Type.TypeID.LIST}, + {"list", Type.TypeID.LIST}, + {"LIST", Type.TypeID.LIST}, + {" list< string > ", Type.TypeID.LIST}, + {"map", Type.TypeID.MAP}, + {"map< string , long >", Type.TypeID.MAP}, + {"map>", Type.TypeID.MAP}, + {"struct", Type.TypeID.STRUCT}, + {"struct>", Type.TypeID.STRUCT}, + {"list>", Type.TypeID.LIST}, + }; + } + + @Test(dataProvider = "validTypes") + public void testParseType(String input, Type.TypeID expectedTypeId) { + Type type = IcebergTypeParser.parseType(input); + assertThat(type.typeId()).isEqualTo(expectedTypeId); + } + + @Test + public void testListElementIsOptional() { + Types.ListType listType = (Types.ListType) IcebergTypeParser.parseType("list"); + assertThat(listType.isElementOptional()).isTrue(); } @Test - public void testPrimitiveDecimal() { - Type type = IcebergTypeParser.parseType("decimal(10,2)"); - assertThat(type).isInstanceOf(Types.DecimalType.class); - Types.DecimalType decimal = (Types.DecimalType) type; + public void testDecimalPrecisionAndScale() { + Types.DecimalType decimal = (Types.DecimalType) IcebergTypeParser.parseType("decimal(10,2)"); assertThat(decimal.precision()).isEqualTo(10); assertThat(decimal.scale()).isEqualTo(2); } @Test - public void testListOfString() { - Type type = IcebergTypeParser.parseType("list"); - assertThat(type).isInstanceOf(Types.ListType.class); - Types.ListType listType = (Types.ListType) type; - assertThat(listType.elementType()).isInstanceOf(Types.StringType.class); - assertThat(listType.isElementOptional()).isTrue(); - } - - @Test - public void testListOfLong() { - Type type = IcebergTypeParser.parseType("list"); - assertThat(type).isInstanceOf(Types.ListType.class); - Types.ListType listType = (Types.ListType) type; - assertThat(listType.elementType()).isInstanceOf(Types.LongType.class); - } - - @Test - public void testMapStringLong() { - Type type = IcebergTypeParser.parseType("map"); - assertThat(type).isInstanceOf(Types.MapType.class); - Types.MapType mapType = (Types.MapType) type; - assertThat(mapType.keyType()).isInstanceOf(Types.StringType.class); - assertThat(mapType.valueType()).isInstanceOf(Types.LongType.class); - } - - @Test - public void testMapWithSpaces() { - Type type = IcebergTypeParser.parseType("map< string , long >"); - assertThat(type).isInstanceOf(Types.MapType.class); - Types.MapType mapType = (Types.MapType) type; - assertThat(mapType.keyType()).isInstanceOf(Types.StringType.class); - assertThat(mapType.valueType()).isInstanceOf(Types.LongType.class); - } - - @Test - public void testSimpleStruct() { - Type type = IcebergTypeParser.parseType("struct"); - assertThat(type).isInstanceOf(Types.StructType.class); - Types.StructType structType = (Types.StructType) type; - assertThat(structType.fields()).hasSize(2); - assertThat(structType.field("a").type()).isInstanceOf(Types.StringType.class); - assertThat(structType.field("b").type()).isInstanceOf(Types.LongType.class); - } - - @Test - public void testNestedListOfStruct() { - Type type = IcebergTypeParser.parseType("list>"); - assertThat(type).isInstanceOf(Types.ListType.class); - Types.ListType listType = (Types.ListType) type; - assertThat(listType.elementType()).isInstanceOf(Types.StructType.class); - Types.StructType inner = (Types.StructType) listType.elementType(); + public void testNestedStructFields() { + Types.ListType list = + (Types.ListType) IcebergTypeParser.parseType("list>"); + Types.StructType inner = (Types.StructType) list.elementType(); assertThat(inner.fields()).hasSize(2); - assertThat(inner.field("id").type()).isInstanceOf(Types.LongType.class); - assertThat(inner.field("name").type()).isInstanceOf(Types.StringType.class); - } - - @Test - public void testMapWithComplexValue() { - Type type = IcebergTypeParser.parseType("map>"); - assertThat(type).isInstanceOf(Types.MapType.class); - Types.MapType mapType = (Types.MapType) type; - assertThat(mapType.keyType()).isInstanceOf(Types.StringType.class); - assertThat(mapType.valueType()).isInstanceOf(Types.ListType.class); - Types.ListType valueList = (Types.ListType) mapType.valueType(); - assertThat(valueList.elementType()).isInstanceOf(Types.IntegerType.class); + assertThat(inner.field("id").type().typeId()).isEqualTo(Type.TypeID.LONG); + assertThat(inner.field("name").type().typeId()).isEqualTo(Type.TypeID.STRING); } - @Test - public void testStructWithNestedList() { - Type type = IcebergTypeParser.parseType("struct>"); - assertThat(type).isInstanceOf(Types.StructType.class); - Types.StructType structType = (Types.StructType) type; - assertThat(structType.field("name").type()).isInstanceOf(Types.StringType.class); - assertThat(structType.field("tags").type()).isInstanceOf(Types.ListType.class); + @DataProvider(name = "invalidTypes") + public Object[][] invalidTypes() { + return new Object[][] { + {"", "must not be empty"}, + {"list<>", "must not be empty"}, + {"map", "exactly 2 type parameters"}, + {"struct", "name:type"}, + {"foobar", "Cannot parse type string"}, + }; } - @Test - public void testCaseInsensitive() { - Type type = IcebergTypeParser.parseType("LIST"); - assertThat(type).isInstanceOf(Types.ListType.class); - Types.ListType listType = (Types.ListType) type; - assertThat(listType.elementType()).isInstanceOf(Types.StringType.class); - } - - @Test - public void testWhitespaceHandling() { - Type type = IcebergTypeParser.parseType(" list< string > "); - assertThat(type).isInstanceOf(Types.ListType.class); - } - - @Test - public void testEmptyTypeStringThrows() { - assertThatThrownBy(() -> IcebergTypeParser.parseType("")) + @Test(dataProvider = "invalidTypes") + public void testParseTypeThrows(String input, String expectedMessage) { + assertThatThrownBy(() -> IcebergTypeParser.parseType(input)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("must not be empty"); - } - - @Test - public void testEmptyListThrows() { - assertThatThrownBy(() -> IcebergTypeParser.parseType("list<>")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("must not be empty"); - } - - @Test - public void testMapWrongArityThrows() { - assertThatThrownBy(() -> IcebergTypeParser.parseType("map")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("exactly 2 type parameters"); - } - - @Test - public void testStructMissingColonThrows() { - assertThatThrownBy(() -> IcebergTypeParser.parseType("struct")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("name:type"); - } - - @Test - public void testInvalidPrimitiveThrows() { - assertThatThrownBy(() -> IcebergTypeParser.parseType("foobar")) - .isInstanceOf(IllegalArgumentException.class); + .hasMessageContaining(expectedMessage); } } From 883adf47d953ab003e5d731165ef5cd16f5784c1 Mon Sep 17 00:00:00 2001 From: kanthi subramanian Date: Mon, 29 Jun 2026 10:36:34 -0400 Subject: [PATCH 3/4] Added integration test for adding complex types to alter table. --- .../run.sh.tmpl | 35 +++++++++++++++++++ .../scenario.yaml | 9 +++++ 2 files changed, 44 insertions(+) create mode 100644 ice-rest-catalog/src/test/resources/scenarios/schema-evolution-complex-types/run.sh.tmpl create mode 100644 ice-rest-catalog/src/test/resources/scenarios/schema-evolution-complex-types/scenario.yaml diff --git a/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-complex-types/run.sh.tmpl b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-complex-types/run.sh.tmpl new file mode 100644 index 00000000..86e51554 --- /dev/null +++ b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-complex-types/run.sh.tmpl @@ -0,0 +1,35 @@ +#!/bin/bash +set -e + +echo "Running schema evolution (complex types) test..." + +SCENARIO_DIR="{{SCENARIO_DIR}}" +INPUT_PATH="${SCENARIO_DIR}/../insert-scan/input.parquet" + +# Setup +{{ICE_CLI}} --config {{CLI_CONFIG}} create-namespace ${NAMESPACE_NAME} +{{ICE_CLI}} --config {{CLI_CONFIG}} insert --create-table ${TABLE_NAME} "file://${INPUT_PATH}" +echo "OK Created table ${TABLE_NAME}" + +# Add list, map, struct columns in one alter-table call +{{ICE_CLI}} --config {{CLI_CONFIG}} alter-table ${TABLE_NAME} \ + '[{"op":"add_column","name":"tags","type":"list"},{"op":"add_column","name":"metadata","type":"map"},{"op":"add_column","name":"address","type":"struct"}]' +echo "OK Altered table: added list, map, struct columns" + +# Verify all three columns appear in the schema +{{ICE_CLI}} --config {{CLI_CONFIG}} describe -s ${TABLE_NAME} > /tmp/schema_ev_complex.txt +for col in tags metadata address; do + if ! grep -q "$col" /tmp/schema_ev_complex.txt; then + echo "FAIL describe -s missing expected column '${col}'" + cat /tmp/schema_ev_complex.txt + exit 1 + fi +done +echo "OK Schema verified: tags, metadata, address present" + +# 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 (complex types) test completed successfully" diff --git a/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-complex-types/scenario.yaml b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-complex-types/scenario.yaml new file mode 100644 index 00000000..f730c266 --- /dev/null +++ b/ice-rest-catalog/src/test/resources/scenarios/schema-evolution-complex-types/scenario.yaml @@ -0,0 +1,9 @@ +name: "Schema evolution - add_column with complex types (list, map, struct)" +description: "Tests alter-table add_column for list, map, and struct" + +catalogConfig: + warehouse: "s3://test-bucket/warehouse" + +env: + NAMESPACE_NAME: "test_schema_ev_complex" + TABLE_NAME: "test_schema_ev_complex.t1" From 3bc5179588d9dd655231a531d2ac4312b7f692c9 Mon Sep 17 00:00:00 2001 From: kanthi subramanian Date: Mon, 29 Jun 2026 23:00:43 -0400 Subject: [PATCH 4/4] Added release notes for 0.17.0 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index da45b4d2..b9d6b64e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.17.0](https://github.com/Altinity/ice/compare/v0.16.0...v0.17.0) + +### Added + +- `ice` - Upgrade iceberg-java to 1.9.2 and support adding required columns ([#145](https://github.com/Altinity/ice/pull/145)) +- `ice` - Add logic to add non-primitive types to alter table add ([#183](https://github.com/Altinity/ice/pull/183)) + ## [0.16.0](https://github.com/Altinity/ice/compare/v0.16.0...v0.15.0) ### Added