RussellSpitzer commented on code in PR #14064:
URL: https://github.com/apache/iceberg/pull/14064#discussion_r2345056550


##########
core/src/test/java/org/apache/iceberg/TestRowLineageMetadata.java:
##########
@@ -190,6 +192,90 @@ public void testFastAppend() {
     assertThat(table.ops().current().nextRowId()).isEqualTo(30 + 17 + 11);
   }
 
+  @TestTemplate
+  public void testFastAppendV1V3() throws IOException {
+    // Test the scenario where a V1 table is upgraded to V3 and then
+    // appends a new Snapshots that includes an old v1 Manifest without 
existingRowsCount.
+
+    // Start with a V1 table
+    TestTables.TestTable table =
+        TestTables.create(
+            tableDir, "test", TEST_SCHEMA, PartitionSpec.unpartitioned(), 1);
+
+    // V1 tables should start with nextRowId = 0 (INITIAL_ROW_ID)
+    assertThat(table.ops().current().nextRowId()).isEqualTo(0L);
+
+    // Manually create a manifest with null existingRowsCount to simulate the 
V1 scenario
+    ManifestFile manifestWithNullExistingRows = new GenericManifestFile(
+        "manifest1.avro", // path
+        100L, // length
+        0, // spec_id
+        ManifestContent.DATA, // content
+        0L, // sequence_number
+        0L, // min_sequence_number
+        1L, // snapshot_id
+        null, // partitions
+        null, // key_metadata
+        1, // added_files_count
+        1L, // added_rows_count
+        null, // existing_files_count
+        null, // existing_rows_count - THIS IS THE KEY: null value
+        0, // deleted_files_count
+        0L, // deleted_rows_count
+        null); // first_row_id
+
+    // Create a manifest list file that contains our custom manifest
+    File manifestListFile = new File(tableDir, "manifest_list_" + 
System.nanoTime() + ".avro");
+    String manifestListLocation;
+    try (ManifestListWriter writer =
+        ManifestLists.write(
+            1, Files.localOutput(manifestListFile), 1L, null, 0, 0L)) {
+      writer.add(manifestWithNullExistingRows);
+      manifestListLocation = Files.localInput(manifestListFile).location();
+    }
+
+    // Create a snapshot with this manifest list
+    Snapshot snapshotWithNullManifest = new BaseSnapshot(
+        0L, // sequence_number
+        1L, // snapshot_id
+        null, // parent_id
+        System.currentTimeMillis(), // timestamp_millis
+        DataOperations.APPEND, // operation
+        null, // summary
+        null, // schema_id
+        manifestListLocation, // manifest_list
+        null, // first_row_id
+        null, // added_rows
+        null); // key_id
+
+    // Add this snapshot to the table metadata
+    TableOperations ops = table.ops();
+    TableMetadata current = ops.current();
+    TableMetadata withSnapshot = TableMetadata.buildFrom(current)
+        .setBranchSnapshot(snapshotWithNullManifest, "main")// Set this as the 
current snapshot
+        .build();
+    ops.commit(current, withSnapshot);
+
+    // Verify that our manually created manifest has null existingRowsCount
+    Snapshot currentSnapshot = table.currentSnapshot();
+    assertThat(currentSnapshot.allManifests(table.io())).isNotEmpty();
+    ManifestFile firstManifest = 
currentSnapshot.allManifests(table.io()).get(0);
+    assertThat(firstManifest.existingRowsCount()).isNull(); // This should now 
be null!
+
+    // Now upgrade the table to V3 - this is where the problem occurs

Review Comment:
   nit: remove "this is where the problem occurs" 



-- 
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

Reply via email to