amogh-jahagirdar commented on code in PR #16688:
URL: https://github.com/apache/iceberg/pull/16688#discussion_r3417987050
##########
core/src/main/java/org/apache/iceberg/TrackedFileStruct.java:
##########
@@ -104,6 +106,7 @@ public PartitionData copy() {
TrackedFileStruct(
Tracking tracking,
FileContent contentType,
+ int writerFormatVersion,
Review Comment:
Is this location in the constructor intentional? Feels a bit weird to have
to pass this in before passing in a location and FileFormat etc?
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -244,8 +285,8 @@ void testProjectedStructLike() {
TrackedFileStruct file = new TrackedFileStruct(projection);
assertThat(file.size()).isEqualTo(2);
- // projected position 0 maps to internal position 2 (location)
- // projected position 1 maps to internal position 5 (file_size_in_bytes)
Review Comment:
Do we really need to change this comment? I know we changed the rest of the
test class but those seemed like a legitimate cleanup. Here, I think we can
avoid some needless conflicts.
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -347,16 +390,17 @@ static TrackedFileStruct createFullTrackedFile() {
new TrackedFileStruct(
tracking,
FileContent.DATA,
+ WRITER_FORMAT_VERSION_V4,
"s3://bucket/data/file.parquet",
FileFormat.PARQUET,
newPartition(7, "music"),
100L,
1024L);
- file.set(6, 0);
- file.set(9, 1);
- file.set(10, dv);
- file.set(12, ByteBuffer.wrap(new byte[] {1, 2, 3}));
- file.set(13, ImmutableList.of(50L));
+ file.set(SPEC_ID_ORDINAL, 0);
+ file.set(SORT_ORDER_ID_ORDINAL, 1);
+ file.set(DELETION_VECTOR_ORDINAL, dv);
+ file.set(KEY_METADATA_ORDINAL, ByteBuffer.wrap(new byte[] {1, 2, 3}));
+ file.set(SPLIT_OFFSETS_ORDINAL, ImmutableList.of(50L));
Review Comment:
Thanks for cleaning all this up!
--
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]
