amogh-jahagirdar commented on code in PR #8520:
URL: https://github.com/apache/iceberg/pull/8520#discussion_r1330824697
##########
core/src/main/java/org/apache/iceberg/BaseTransaction.java:
##########
@@ -421,6 +421,7 @@ private void commitSimpleTransaction() {
.onlyRetryOn(CommitFailedException.class)
.run(
underlyingOps -> {
+ deletedFiles.clear();
Review Comment:
Yes you're right, I stepped through the code a bit more, really the
applyUpdates is more of a `reapplyUpdates` (in case the table state changed).
`applyUpdates` is a no-op on the first attempt. If the deletedFiles set is
cleared before the first commit and that commit succeeds we would never end up
cleaning those files based on the cleanup done here
https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/BaseTransaction.java#L450.
Although I don't know why our tests wouldn't pick up on this, we have tests
which validate we cleanup the files we would expect
--
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]
