RussellSpitzer commented on code in PR #12672:
URL: https://github.com/apache/iceberg/pull/12672#discussion_r2019486483
##########
core/src/test/java/org/apache/iceberg/TestManifestListVersions.java:
##########
@@ -150,25 +163,140 @@ public void testV2Write() throws IOException {
assertThat(manifest.deletedRowsCount()).isEqualTo(DELETED_ROWS);
}
+ @Test
+ public void testV3Write() throws IOException {
+ ManifestFile manifest = writeAndReadManifestList(3);
+
+ // all v3 fields should be read correctly
+ assertThat(manifest.path()).isEqualTo(PATH);
+ assertThat(manifest.length()).isEqualTo(LENGTH);
+ assertThat(manifest.partitionSpecId()).isEqualTo(SPEC_ID);
+ assertThat(manifest.content()).isEqualTo(ManifestContent.DATA);
+ assertThat(manifest.sequenceNumber()).isEqualTo(SEQ_NUM);
+ assertThat(manifest.minSequenceNumber()).isEqualTo(MIN_SEQ_NUM);
+ assertThat(manifest.snapshotId()).isEqualTo(SNAPSHOT_ID);
+ assertThat(manifest.addedFilesCount()).isEqualTo(ADDED_FILES);
+ assertThat(manifest.addedRowsCount()).isEqualTo(ADDED_ROWS);
+ assertThat(manifest.existingFilesCount()).isEqualTo(EXISTING_FILES);
+ assertThat(manifest.existingRowsCount()).isEqualTo(EXISTING_ROWS);
+ assertThat(manifest.deletedFilesCount()).isEqualTo(DELETED_FILES);
+ assertThat(manifest.deletedRowsCount()).isEqualTo(DELETED_ROWS);
+ assertThat(manifest.firstRowId()).isEqualTo(FIRST_ROW_ID);
+ }
+
+ @Test
+ public void testV3WriteFirstRowIdAssignment() throws IOException {
+ ManifestFile missingFirstRowId =
+ new GenericManifestFile(
+ PATH,
+ LENGTH,
+ SPEC_ID,
+ ManifestContent.DATA,
+ SEQ_NUM,
+ MIN_SEQ_NUM,
+ SNAPSHOT_ID,
+ PARTITION_SUMMARIES,
+ KEY_METADATA,
+ ADDED_FILES,
+ ADDED_ROWS,
+ EXISTING_FILES,
+ EXISTING_ROWS,
+ DELETED_FILES,
+ DELETED_ROWS,
+ null);
+
+ // write uses firstRowId=WRITER_FIRST_ROW_ID and ADDED_ROWS are assigned
+ long nextRowId = WRITER_FIRST_ROW_ID + ADDED_ROWS;
+ ManifestFile manifest =
+ Iterables.getOnlyElement(
+ ManifestLists.read(writeManifestList(3, nextRowId,
missingFirstRowId)));
+
+ // all v3 fields should be read correctly
+ assertThat(manifest.path()).isEqualTo(PATH);
+ assertThat(manifest.length()).isEqualTo(LENGTH);
+ assertThat(manifest.partitionSpecId()).isEqualTo(SPEC_ID);
+ assertThat(manifest.content()).isEqualTo(ManifestContent.DATA);
+ assertThat(manifest.sequenceNumber()).isEqualTo(SEQ_NUM);
+ assertThat(manifest.minSequenceNumber()).isEqualTo(MIN_SEQ_NUM);
+ assertThat(manifest.snapshotId()).isEqualTo(SNAPSHOT_ID);
+ assertThat(manifest.addedFilesCount()).isEqualTo(ADDED_FILES);
+ assertThat(manifest.addedRowsCount()).isEqualTo(ADDED_ROWS);
+ assertThat(manifest.existingFilesCount()).isEqualTo(EXISTING_FILES);
+ assertThat(manifest.existingRowsCount()).isEqualTo(EXISTING_ROWS);
+ assertThat(manifest.deletedFilesCount()).isEqualTo(DELETED_FILES);
+ assertThat(manifest.deletedRowsCount()).isEqualTo(DELETED_ROWS);
+ assertThat(manifest.firstRowId()).isEqualTo(WRITER_FIRST_ROW_ID);
+ }
+
+ @Test
+ public void testV3WriteMixedRowIdAssignment() throws IOException {
+ ManifestFile missingFirstRowId =
+ new GenericManifestFile(
+ PATH,
+ LENGTH,
+ SPEC_ID,
+ ManifestContent.DATA,
+ SEQ_NUM,
+ MIN_SEQ_NUM,
+ SNAPSHOT_ID,
+ PARTITION_SUMMARIES,
+ KEY_METADATA,
+ ADDED_FILES,
+ ADDED_ROWS,
+ EXISTING_FILES,
+ EXISTING_ROWS,
+ DELETED_FILES,
+ DELETED_ROWS,
+ null);
+
+ // write uses firstRowId=WRITER_FIRST_ROW_ID and ADDED_ROWS are assigned
twice
+ long nextRowId = WRITER_FIRST_ROW_ID + ADDED_ROWS + ADDED_ROWS;
+ List<ManifestFile> manifests =
+ ManifestLists.read(
+ writeManifestList(3, nextRowId, missingFirstRowId, TEST_MANIFEST,
missingFirstRowId));
+
+ // all v2 fields should be read correctly
+ for (ManifestFile manifest : manifests) {
+ assertThat(manifest.path()).isEqualTo(PATH);
+ assertThat(manifest.length()).isEqualTo(LENGTH);
+ assertThat(manifest.partitionSpecId()).isEqualTo(SPEC_ID);
+ assertThat(manifest.content()).isEqualTo(ManifestContent.DATA);
+ assertThat(manifest.sequenceNumber()).isEqualTo(SEQ_NUM);
+ assertThat(manifest.minSequenceNumber()).isEqualTo(MIN_SEQ_NUM);
+ assertThat(manifest.snapshotId()).isEqualTo(SNAPSHOT_ID);
+ assertThat(manifest.addedFilesCount()).isEqualTo(ADDED_FILES);
+ assertThat(manifest.addedRowsCount()).isEqualTo(ADDED_ROWS);
+ assertThat(manifest.existingFilesCount()).isEqualTo(EXISTING_FILES);
+ assertThat(manifest.existingRowsCount()).isEqualTo(EXISTING_ROWS);
+ assertThat(manifest.deletedFilesCount()).isEqualTo(DELETED_FILES);
+ assertThat(manifest.deletedRowsCount()).isEqualTo(DELETED_ROWS);
+ }
+
+ assertThat(manifests.get(0).firstRowId()).isEqualTo(WRITER_FIRST_ROW_ID);
+ assertThat(manifests.get(1).firstRowId()).isEqualTo(FIRST_ROW_ID);
Review Comment:
Could we check against TEST_MANIFEST.firstRowId()?
I'm looking to see how we can make this more semantically
"The added test manifest should have the same row id that it had originally"
--
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
