gaborkaszab commented on code in PR #16688:
URL: https://github.com/apache/iceberg/pull/16688#discussion_r3467418060
##########
core/src/main/java/org/apache/iceberg/TrackedFile.java:
##########
@@ -35,6 +35,9 @@ interface TrackedFile {
"content_type",
Types.IntegerType.get(),
"Type of content: 0=DATA, 2=EQUALITY_DELETES, 3=DATA_MANIFEST,
4=DELETE_MANIFEST");
+ Types.NestedField WRITER_FORMAT_VERSION =
+ Types.NestedField.required(
+ 157, "writer_format_version", Types.IntegerType.get(), "Writer
format version");
Review Comment:
Thanks for the feedback @rdblue and @amogh-jahagirdar !
The points discussed so far:
1) I'll open a follow-up PR to rename the field to `format_version`
2) required + 0 or optional + null for pre-v4 written files
I don't have a strong opinion, either seems fine. Maybe required + 0 forces
the users/writers to think through more what value to write?
3) Will keep the field in `TrackedFile` instead of moving to `ManifestInfo`
4) enum instead of int
I think forward-compatibility is still an open question for the enum
representation. For instance in a V4 reader the enum could look like this:
```
enum FormatVersion {
PRE_V4(0),
V4(4);
...
public static FormatVersion fromId(int id) { ... }
}
```
What would be the expectation of reading metadata written by V5 writers,
like `FormatVersion.fromId(5);`?
Should we keep an enum value for a later unknown version? It'd be "lossy" as
it won't necessary keep the original value. We could have a class instead of an
enum that can also keep the original value and also provide `fromId(int)` and
`greaterThan(int)` functions.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
