amogh-jahagirdar commented on code in PR #11131: URL: https://github.com/apache/iceberg/pull/11131#discussion_r1815720545
########## core/src/test/java/org/apache/iceberg/TestRowDelta.java: ########## @@ -595,18 +601,18 @@ public void testDeleteDataFileWithDeleteFile() { assertThat(deleteSnap.deleteManifests(table.io())).hasSize(1); validateDeleteManifest( deleteSnap.deleteManifests(table.io()).get(0), - dataSeqs(1L), - fileSeqs(1L), - ids(deltaSnapshotId), - files(FILE_A_DELETES), - statuses(Status.ADDED)); + dataSeqs(1L, 1L), + fileSeqs(1L, 1L), + ids(deltaSnapshotId, deltaSnapshotId), + files(FILE_A_DELETES, FILE_B_DELETES), + statuses(Status.ADDED, Status.ADDED)); Review Comment: See https://github.com/apache/iceberg/pull/11131#discussion_r1815710103 for why I changed this test. TLDR, the behavior around rewriting manifests with aged off deletes changed from "Always rewrite in case the manifest does have aged off deletes" to "Remove the aged off deletes if the manifest actually needs to be rewritten for other purposes". This test used to just work on the assumption that this condition existed, but now we need to perform an action (deleting FILE_B_DELETES) which forces the manifest to be rewritten along with the deleted, aged off entries. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org