rdblue commented on code in PR #16936:
URL: https://github.com/apache/iceberg/pull/16936#discussion_r4020756708
##########
core/src/main/java/org/apache/iceberg/TrackedFileStruct.java:
##########
@@ -267,27 +267,31 @@ public TrackedFile copyWithStats(Set<Integer>
requestedColumnIds) {
@Override
protected <T> T internalGet(int pos, Class<T> javaClass) {
- return javaClass.cast(getByPos(pos));
+ return javaClass.cast(getByPos(this, pos));
}
- private Object getByPos(int pos) {
+ /**
+ * Returns the value at the given position in TrackedFile schema field
order, shared with the
+ * write-direction wrappers so that the position-to-field mapping is defined
in one place.
+ */
+ static Object getByPos(TrackedFile file, int pos) {
Review Comment:
I don't think that it makes sense to share this method. Looks like the
motivation is to implement a wrapper that can be written into a manifest file,
but that is the responsibility of version-specific wrappers that live in
`V4Metadata` and should be kept separate.
The `get` and `set` methods in this class cover all fields of the current
version of the structure that we use in memory. For example,
`GenericManifestEntry` has v1 fields like `snapshot`, v2 fields like
`sequence_number`, and v3 fields like `first_row_id`. When reading, the
manifest's file schema is used as a projection and this class maps that schema
into the current set of in-memory fields.
In the write path, we have version-specific wrappers that do the opposite
and are also responsible for validating the version that is being written. The
fields that are mapped in the version-specific wrappers will differ from the
fields here as fields are added. We don't go back and add new fields like v2
`sequence_number` to v1 write adapters.
--
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]