danielcweeks commented on code in PR #12250: URL: https://github.com/apache/iceberg/pull/12250#discussion_r1958620415
########## spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/RewritePositionDeleteFilesSparkAction.java: ########## @@ -140,6 +148,33 @@ public RewritePositionDeleteFiles.Result execute() { } } + private boolean hasDeletionVectors() { Review Comment: I don't feel like we're scanning for the right thing here. Shouldn't we be checking to see if there are any positional delete files here (not puffin DVs)? If this is run on a V3 table, it shouldn't matter whether there are already DV files, only that that there are positional delete files that need to be rewritten. This appears to assume that if there is a single DV, that everything has already been rewritten, which is not the case. PosDel and DVs can coexist when a table is upgraded to v3. That said, this should probably be called something like `requiresDVRewrite()` -- 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