openinx commented on a change in pull request #2510:
URL: https://github.com/apache/iceberg/pull/2510#discussion_r619939799
##########
File path: core/src/main/java/org/apache/iceberg/AllManifestsTable.java
##########
@@ -44,15 +44,16 @@
Types.NestedField.required(1, "path", Types.StringType.get()),
Types.NestedField.required(2, "length", Types.LongType.get()),
Types.NestedField.optional(3, "partition_spec_id",
Types.IntegerType.get()),
- Types.NestedField.optional(4, "added_snapshot_id", Types.LongType.get()),
- Types.NestedField.optional(5, "added_data_files_count",
Types.IntegerType.get()),
- Types.NestedField.optional(6, "existing_data_files_count",
Types.IntegerType.get()),
- Types.NestedField.optional(7, "deleted_data_files_count",
Types.IntegerType.get()),
- Types.NestedField.optional(8, "partition_summaries",
Types.ListType.ofRequired(9, Types.StructType.of(
- Types.NestedField.required(10, "contains_null",
Types.BooleanType.get()),
- Types.NestedField.required(11, "contains_nan",
Types.BooleanType.get()),
- Types.NestedField.optional(12, "lower_bound",
Types.StringType.get()),
- Types.NestedField.optional(13, "upper_bound", Types.StringType.get())
+ Types.NestedField.optional(4, "content", Types.IntegerType.get()),
Review comment:
The correct way to add a new field in this manifest table is : add the
`content` field with an auto-increment field id (which should be 14 in this
case) at the tail of those nested field list. We cannot change the existing
field ids in this schema because them are mapping to the underlying rows in
generated avro files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
