aokolnychyi commented on code in PR #6682: URL: https://github.com/apache/iceberg/pull/6682#discussion_r1095234465
########## api/src/main/java/org/apache/iceberg/actions/DeleteOrphanFiles.java: ########## @@ -80,9 +84,16 @@ public interface DeleteOrphanFiles extends Action<DeleteOrphanFiles, DeleteOrpha * * @param executorService the service to use * @return this for method chaining + * @deprecated All deletes should be performed using the bulk delete api if available. Use FileIO + * specific parallelism controls to adjust bulk delete concurrency within that api. */ + @Deprecated DeleteOrphanFiles executeDeleteWith(ExecutorService executorService); + default DeleteOrphanFiles deleteBulkWith(Consumer<Iterable<String>> deleteFunc) { Review Comment: My worry is that the current implementation breaks the behavior in an incompatible way: it checks if the table IO supports bulk deletes and if yes, ignores `deleteWith`. It will break those dry-run scenarios if the table is configured with a non-bulk IO and the user passes a custom delete function. Can you elaborate a bit on why this is needed? Why not just go through the non-bulk delete API if a custom delete function is provided? -- 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