nastra commented on code in PR #11273: URL: https://github.com/apache/iceberg/pull/11273#discussion_r1799303996
########## core/src/main/java/org/apache/iceberg/util/ContentFileUtil.java: ########## @@ -80,4 +80,8 @@ public static String referencedDataFileLocation(DeleteFile deleteFile) { CharSequence location = referencedDataFile(deleteFile); return location != null ? location.toString() : null; } + + public static boolean isFileScopedDelete(DeleteFile deleteFile) { Review Comment: nit: might be worth updating https://github.com/apache/iceberg/blob/c8fe01e71f9996fcaae973c6bfaa8d90f7dd8c6c/core/src/main/java/org/apache/iceberg/deletes/SortingPositionOnlyDeleteWriter.java#L174 to use this method too ########## spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMergeOnReadDelete.java: ########## @@ -99,6 +101,76 @@ public void testDeletePartitionGranularity() throws NoSuchTableException { checkDeleteFileGranularity(DeleteGranularity.PARTITION); } + @TestTemplate + public void testPositionDeletesAreMaintainedDuringDelete() throws NoSuchTableException { + String partitionStmt = "PARTITIONED BY (id)"; + sql( + "CREATE TABLE %s (id int, data string) USING iceberg %s TBLPROPERTIES" + + "('format-version'='2', 'write.delete.mode'='merge-on-read', 'write.delete.granularity'='file')", Review Comment: nit: in `checkDeleteFileGranularity()` we're using `TableProperties` rather than plain strings, so I think it would be good to do the same here too -- 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