RussellSpitzer commented on code in PR #16285:
URL: https://github.com/apache/iceberg/pull/16285#discussion_r4339567748
##########
core/src/test/java/org/apache/iceberg/TestV4ManifestReader.java:
##########
@@ -172,7 +172,8 @@ public void readDataFile(FileFormat format) throws
IOException {
DV,
null, // manifest info
ByteBuffer.wrap(new byte[] {1, 2, 3}), // key metadata
- ImmutableList.of(50L, 100L));
+ ImmutableList.of(50L, 100L),
+ null); // column files
Review Comment:
I would argue the "relative paths" here is actually not being done the
correct way either. Even so that's a bit of a tiny change and if we wanted to
do that now I wouldn't mind. I think what we really should be doing there is
passing transformers directly to the readers themselves so they can fix these
fields at read time rather than modifying the object afterwards. Would love to
check with @rdblue on his thoughts on this. Doing the "resolution" as an
afterthought feels messy to me. Thats I think why adding the code for that
feels "complex" because it is actually doing something at the wrong layer.
So something like
reader
.transformField(field, resolvePath(tableRoot))
--
I'd still just do everything in this PR and cite the previous
"relativeLocation" as an example.
--
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]
