jackye1995 commented on a change in pull request #2510:
URL: https://github.com/apache/iceberg/pull/2510#discussion_r620837466
##########
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:
I would prefer it to be a String field, showing content 0 or 1 does not
make too much sense from user perspective, comparing to showing `DATA` or
`DELETES`
##########
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:
Prefer to add it as the last entry so that ID of other fields are not
notified for backwards compatibility.
--
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]
