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


##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteDataFilesProcedure.java:
##########
@@ -976,19 +978,67 @@ public void testRewriteDataFilesSummary() {
             EnvironmentContext.ENGINE_VERSION, v -> 
assertThat(v).startsWith("4.0"));
   }
 
+  @TestTemplate

Review Comment:
   @jeesou For a compaction both cases will end up going through the 
`SparkWrite` (the compaction of MoR case is undifferentiated in that the task 
groupings will contain the delete files to apply, and the output rows will just 
be the original rows - any marked as deleted).
   
   That said, I do think it's probably good to have separate explicit cases in 
case someone refactors and changes the compaction path, we can catch potential 
issues that way. 



##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteDataFilesProcedure.java:
##########
@@ -976,19 +978,67 @@ public void testRewriteDataFilesSummary() {
             EnvironmentContext.ENGINE_VERSION, v -> 
assertThat(v).startsWith("4.0"));
   }
 
+  @TestTemplate

Review Comment:
   @jeesou For a compaction both cases will end up going through the 
`SparkWrite` (the compaction of MoR case is undifferentiated in that the task 
groupings will contain the delete files to apply, and the output rows will just 
be the original rows - any marked as deleted).
   
   That said, I do think it's probably good to have separate explicit tests in 
case someone refactors and changes the compaction path, we can catch potential 
issues that way. 



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