Added logic to create table from schema. - #219
Conversation
c8b7049 to
6b6e467
Compare
e238b47 to
0144dd4
Compare
4a93449 to
7d50554
Compare
|
| } | ||
|
|
||
| if (schemaJson != null && !schemaJson.isEmpty()) { | ||
| CreateTable.run( |
There was a problem hiding this comment.
wanted to not to touch the current flow of CreateTable
xieandrew
left a comment
There was a problem hiding this comment.
There's an issue with field id ordering with nested schemas plus some other comments.
Also, not exactly from this PR but I noticed that nested fields don't support the required option, in IcebergTypeParser it always uses optional. Not sure if that belongs in this PR or could be added later.
| int fieldId = nextId.incrementAndGet(); | ||
| Type type = IcebergTypeParser.parseType(field.type(), nextId); | ||
| boolean required = field.required() != null && field.required(); | ||
| columns.add( | ||
| required | ||
| ? Types.NestedField.required(fieldId, field.name(), type, field.doc()) | ||
| : Types.NestedField.optional(fieldId, field.name(), type, field.doc())); | ||
| } | ||
| return new Schema(columns); |
There was a problem hiding this comment.
The field id ordering from this method doesn't match what the metadata actually creates for nested schemas with struct. For example ice create-table schema.nested --schema '[{"name":"a","type":"struct<x:string,y:long>"},{"name":"b","type":"long"}]' results in "a": 1, "x": 3, "y": 4, "b": 2 in the real table metadata but this method produces "a": 1, "x": 2, "y": 3, "b": 4 which ends up in the table property "schema.name-mapping.default" (mismatched from the real schema). I think the fix is to add TypeUtil.assignIncreasingFreshIds(schema) before the return.
| } | ||
|
|
||
| private static Type parseType(String typeString, AtomicInteger nextId) { | ||
| public static Type parseType(String typeString, AtomicInteger nextId) { |
There was a problem hiding this comment.
Create table with variant type does not work: ice create-table schema.variant --schema '[{"name":"v","type":"variant"}]'
Error:
java.lang.IllegalArgumentException: Cannot parse type string: variant is not a primitive type
at org.apache.iceberg.types.Types.fromPrimitiveString(Types.java:116)
at com.altinity.ice.cli.internal.util.IcebergTypeParser.parseType(IcebergTypeParser.java:52)
at com.altinity.ice.cli.internal.util.IceSchemaParser.toSchema(IceSchemaParser.java:77)
at com.altinity.ice.cli.internal.util.IceSchemaParser.parse(IceSchemaParser.java:56)
at com.altinity.ice.cli.Main.createTable(Main.java:403)
at com.altinity.ice.cli.Main.lambda$main$4(Main.java:1258) [9 skipped]
at com.altinity.ice.cli.Main.main(Main.java:1265) [1 skipped]
| formatVersion, | ||
| partitions, | ||
| sortOrders); | ||
| if ((schemaFile == null) == (schemaJson == null || schemaJson.isEmpty())) { |
There was a problem hiding this comment.
I think schemaFile should check isEmpty too since --schema-from-parquet "" with empty string would currently be allowed.
| Map<Integer, Types.NestedField> byId = TypeUtil.indexById(schema.asStruct()); | ||
| // a, x, y, b, list element | ||
| assertThat(byId).hasSize(5); | ||
| assertThat(byId.keySet()).containsExactlyInAnyOrder(1, 2, 3, 4, 5); |
There was a problem hiding this comment.
This should check the exact order not just a set to verify the ordering issue from the comment in IceSchemaParser.
…ID order in IceSchemaParserTest
…ds to match catalog
…moved from quay.io/dockerhub)
closes: #218
Add support for creating table by passing schema.