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


##########
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 and agree that it makes sense to add size attribute to rewrite 
result, but the way `RewriteResult` is currently implemented and used is 
counterintuitive to me. When I see `RewriteResult<T>`, I expect the result of 
rewriting `T`. Adding another general-purpose and nullable attribute that's not 
related to `T` makes it even more confusing. I did consider doing exactly that 
initially, but it felt like shoehorning, attempting to attach metadata about 
the container (the manifest file's size) to an object that describes its 
contents. Same for `RewriteContentFileResult`, which is public too.
   
   That said, if we're aligned that modifying `RewriteResult` is the preferred 
path forward, I'm happy to implement it that way.



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