amogh-jahagirdar commented on code in PR #16699:
URL: https://github.com/apache/iceberg/pull/16699#discussion_r3377076009


##########
spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteManifestsAction.java:
##########
@@ -196,6 +196,144 @@ public void testRewriteManifestsPreservesOptionalFields() 
throws IOException {
     }
   }
 
+  @TestTemplate
+  public void testRewriteV3ManifestsPreservesFirstRowId() {
+    assumeThat(formatVersion).isGreaterThanOrEqualTo(3);
+
+    PartitionSpec spec = PartitionSpec.unpartitioned();
+    Map<String, String> options = Maps.newHashMap();
+    options.put(TableProperties.FORMAT_VERSION, String.valueOf(formatVersion));
+    Table table = TABLES.create(SCHEMA, spec, options, tableLocation);
+
+    writeRecords(Lists.newArrayList(new ThreeColumnRecord(1, null, "AAAA")));
+    writeRecords(Lists.newArrayList(new ThreeColumnRecord(2, "CCCC", "CCCC")));
+    table.refresh();
+
+    assertThat(table.currentSnapshot().dataManifests(table.io())).hasSize(2);
+
+    List<Row> rowsBefore =
+        spark
+            .read()
+            .format("iceberg")
+            .load(tableLocation)
+            .selectExpr("_row_id", "_last_updated_sequence_number", "*")
+            .orderBy("_row_id")
+            .collectAsList();

Review Comment:
   So I took a look, we'd have to move currentDataWithLineage() out of 
TestRewriteDataFilesAction into TestBase, which (a) currently needs a 
tableLocation param since TestBase has no such field and (b) modifies an 
unrelated test file in what's meant to be a focused manifest-rewrite fix (and 
avoid backporting test refactorings). I'd rather keep the reads inline here and 
leave that consolidation to a separate cleanup if we want it. 



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

Reply via email to