gaborkaszab commented on code in PR #16285:
URL: https://github.com/apache/iceberg/pull/16285#discussion_r4336093200
##########
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:
@RussellSpitzer @stevenzwu @anoopj
I've rebased [my PR that wires
in](https://github.com/apache/iceberg/pull/17826/changes/8715fe8635088f3792e841ef87050a84e3ce3ecf)
`ColumnFile`s into the `V4ManifestReader`. For me it seems too much
complexity to add on top of this. It's not just setting the custom type, but
also resolving relative paths (with adding functionality setting the location
on a column file), and extending comparators, test helpers, adding new test
coverage, etc.
Added a link, if you still feel it should be included in this one, then I
can, but I'm very hesitant seeing the size and complexity of the reader
integration compared to this PR.
--
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]
