dramaticlly commented on code in PR #13720:
URL: https://github.com/apache/iceberg/pull/13720#discussion_r2291912926


##########
core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java:
##########
@@ -447,6 +551,58 @@ public static RewriteResult<DeleteFile> 
rewriteDeleteManifest(
     }
   }
 
+  /**
+   * Rewrite a delete manifest, replacing path references.
+   *
+   * @param manifestFile source delete manifest to rewrite
+   * @param snapshotIds snapshot ids for filtering returned delete manifest 
entries
+   * @param outputFile output file to rewrite manifest file to
+   * @param io file io
+   * @param format format of the manifest file
+   * @param specsById map of partition specs by id
+   * @param sourcePrefix source prefix that will be replaced
+   * @param targetPrefix target prefix that will replace it
+   * @param stagingLocation staging location for rewritten files (referred 
delete file will be
+   *     rewritten here)
+   * @return size of the resulting manifest file and a copy plan for the 
referenced content files
+   */
+  public static Pair<Long, RewriteResult<DeleteFile>> 
rewriteDeleteManifestWithResult(

Review Comment:
   I understand your confusion and I think we can probably do better on this 
RewriteResult<T> class. It's counterintuitive in a way that the instance of 
this result is not just for rewrite of T, but a collection T require 
open/rewrite/close in next iteration and also a collection of file path to copy 
from source to destination. 
   
   Take a manifestList as an example, this avro file contain mapping to many 
manifests. So when we trying to path rewrite a manifestList, it contains list 
of manifests require open to rewrite, list of source to destination path to 
plan the copy. Conceptually I think the result is relate to the manifest-list, 
now we are adding the size of written manifest lists. Recursively this also 
applies to when we apply path rewrite on a data manifest level, results contain 
set of data files, list of copy plans and size of the manifest. 
   
   For existing rewriteManifestList method in utils, the return type is of 
`RewriteResult<ManifestFile>` instead of `RewriteResult<ManifestListFile>`. Ans 
similarly the rewriteVersionFiles are returning RewriteResult<Snapshot>. 



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

Reply via email to