aokolnychyi commented on code in PR #7651:
URL: https://github.com/apache/iceberg/pull/7651#discussion_r1199490813


##########
api/src/main/java/org/apache/iceberg/RewriteFiles.java:
##########
@@ -97,6 +97,22 @@ default RewriteFiles addFile(DeleteFile deleteFile) {
         this.getClass().getName() + " does not implement addFile");
   }
 
+  /**
+   * Add a new delete file with the given data sequence number.
+   *
+   * <p>This rewrite operation may change the size or layout of the delete 
files. When applicable,
+   * it is also recommended to discard delete records for files that are no 
longer part of the table
+   * state. However, the set of applicable delete records must never change.
+   *
+   * @param deleteFile a new delete file
+   * @param dataSequenceNumber data sequence number to append on the file
+   * @return this for method chaining
+   */
+  default RewriteFiles addFile(DeleteFile deleteFile, long dataSequenceNumber) 
{

Review Comment:
   > Right now, we don't have a way to set the sequence number in the public 
API.
   
   That's can be easily changed by extending `TableMetadata$Builder`. I am not 
a big fan of `DeleteFileHolder` and I do think places that actually set data 
sequence numbers (like rewrite position deletes) would benefit from passing it 
as part of delete file object. That said, I am concerned about edge cases when 
the passed object may contain a wrong data sequence number. Are there use cases 
we can think of? Like cherry-picks maybe?
   
   



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