[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #2303: Core: Remove all delete files in RewriteFiles action.

[GitBox <gi...@apache.org>]

RussellSpitzer commented on a change in pull request #2303:
URL: https://github.com/apache/iceberg/pull/2303#discussion_r609763716



##########
File path: core/src/main/java/org/apache/iceberg/actions/BaseRewriteDataFilesAction.java
##########
@@ -262,31 +269,64 @@ public RewriteDataFilesActionResult execute() {
     return tasksGroupedByPartition.asMap();
   }
 
-  private void replaceDataFiles(Iterable<DataFile> deletedDataFiles, Iterable<DataFile> addedDataFiles) {
+  private void replaceDataFiles(RewriteResult result) {
     try {
       RewriteFiles rewriteFiles = table.newRewrite();
-      rewriteFiles.rewriteFiles(Sets.newHashSet(deletedDataFiles), Sets.newHashSet(addedDataFiles));
+
+      rewriteFiles.rewriteFiles(
+          Sets.newHashSet(result.dataFilesToDelete()),
+          Sets.newHashSet(result.deleteFilesToDelete()),
+          Sets.newHashSet(result.dataFilesToAdd()),
+          Sets.newHashSet(result.deleteFilesToAdd())
+      );
+
       commit(rewriteFiles);
     } catch (Exception e) {
-      Tasks.foreach(Iterables.transform(addedDataFiles, f -> f.path().toString()))
+      // Remove all the newly produced files if possible.
+      Iterable<ContentFile<?>> addedFiles = Iterables.concat(
+          Arrays.asList(result.dataFilesToAdd()),
+          Arrays.asList(result.deleteFilesToAdd())
+      );
+
+      Tasks.foreach(Iterables.transform(addedFiles, f -> f.path().toString()))
           .noRetry()
           .suppressFailureWhenFinished()
           .onFailure((location, exc) -> LOG.warn("Failed to delete: {}", location, exc))
           .run(fileIO::deleteFile);
+
       throw e;
     }
   }
 
-  private boolean isPartialFileScan(CombinedScanTask task) {
+  private boolean doPartitionNeedRewrite(Collection<FileScanTask> partitionTasks) {
+    int files = 0;
+    for (FileScanTask scanTask : partitionTasks) {

Review comment:
       I believe this would break splitting large files, since a file scan task with only a single file will never be marked for rewrite.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org