stevenzwu commented on code in PR #16936:
URL: https://github.com/apache/iceberg/pull/16936#discussion_r3709501721
##########
core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java:
##########
@@ -425,4 +980,80 @@ private static PartitionSpec resolveSpec(
throw new IllegalArgumentException(
"Cannot find unpartitioned spec in specs: " + specsById.keySet());
}
+
+ // Presents a TrackedFile as its persisted StructLike, shared by the
reusable write-direction
+ // wrappers.
+ private static Object getByPos(TrackedFile file, int pos) {
Review Comment:
Worth fixing. One clarification on the failure mode: adding a field is the
loud case — `size()` derives from `TrackedFile.schema()` on both sides, so a
new ordinal falls through to `default -> throw`. Reordering is the silent one:
sizes still match, every ordinal resolves, and values land in the wrong columns.
The mapping shouldn't live on `TrackedFile` — positional access is a
`StructLike` concern the interface doesn't model. `TrackedFileStruct` is the
better home, since it owns `BASE_TYPE`, which defines the ordinal domain. Plan
is a package-private `static Object getByPos(TrackedFile file, int pos)` there,
called from both. The dependency then runs adapter -> struct, so the structs
keep no dependency on the adapter.
Close to mechanical, since the copy here is already `static
getByPos(TrackedFile, int)` invoked as `getByPos(this, pos)`, and the struct's
accessors for ordinals 0-12 are plain field returns (`partition()` returns
`partitionData`, exactly what `case 8` reads), with 13-15 already delegating to
accessors.
Consolidating also settles the coverage question without new tests:
`TestTrackedFileStruct.getByPosition` already resolves all 16 positions by
field name from the schema, so the single remaining switch stays pinned to the
schema order. Worth noting `internalSet` and the hand-written `BASE_TYPE` still
restate that order separately, so this removes one duplicate encoding rather
than all of them — though both then sit next to the consolidated getter.
--
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]
