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" 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..8b023eec --- /dev/null +++ b/ice/src/main/java/com/altinity/ice/cli/internal/util/IcebergTypeParser.java @@ -0,0 +1,156 @@ +/* + * 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..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 @@ -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,144 @@ 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..ac355b29 --- /dev/null +++ b/ice/src/test/java/com/altinity/ice/cli/internal/util/IcebergTypeParserTest.java @@ -0,0 +1,92 @@ +/* + * 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.DataProvider; +import org.testng.annotations.Test; + +public class IcebergTypeParserTest { + + @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 testDecimalPrecisionAndScale() { + Types.DecimalType decimal = (Types.DecimalType) IcebergTypeParser.parseType("decimal(10,2)"); + assertThat(decimal.precision()).isEqualTo(10); + assertThat(decimal.scale()).isEqualTo(2); + } + + @Test + 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().typeId()).isEqualTo(Type.TypeID.LONG); + assertThat(inner.field("name").type().typeId()).isEqualTo(Type.TypeID.STRING); + } + + @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(dataProvider = "invalidTypes") + public void testParseTypeThrows(String input, String expectedMessage) { + assertThatThrownBy(() -> IcebergTypeParser.parseType(input)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(expectedMessage); + } +}