stevenzwu commented on code in PR #16285:
URL: https://github.com/apache/iceberg/pull/16285#discussion_r3741920853
##########
core/src/test/java/org/apache/iceberg/TestTrackingStruct.java:
##########
@@ -175,10 +204,22 @@ void doNotInheritSequenceNumberForModifiedEntries() {
assertThat(tracking.fileSequenceNumber()).isEqualTo(6L);
}
+ @Test
+ void inheritDataSequenceNumberAfterColumnFilesChange() {
+ // Adding column files should set data sequence number to null
Review Comment:
I thought the idea is to avoid creating Builder only for testing purpose. If
builder is legitimately needed, I don't see why I can't use them in the test
code.
But since `columnFilesUpdated` API is already tested in
`TestTrackingBuilder`, this is fine. The comment seems to suggest that we
should construct the tracking with `columnFilesUpdated`, which led me to my
earlier thought. Maybe we should remove the comment. Also maybe rename the test
method name to `inheritDataSequenceNumberWithModifiedStatus`, since this is
strictly testing inheritance for modified status. There is no concept of column
file change in this test.
--
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]
