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