zeroshade commented on code in PR #2309:
URL: https://github.com/apache/iceberg-go/pull/2309#discussion_r4391064374
##########
table/transaction.go:
##########
@@ -1818,21 +1818,18 @@ type DeleteFileAddition struct {
DataSequenceNumber int64
}
-// ReplaceFiles atomically replaces data files and removes associated delete
files
-// in a single snapshot. This is the commit primitive for compaction: old data
files
-// are replaced with new (compacted) data files, and delete files that are
fully
-// applied are removed. On v3 a removed data file's live deletion vector is
-// removed with it, whether or not it is in deleteFilesToRemove.
-func (t *Transaction) ReplaceFiles(ctx context.Context, dataFilesToDelete,
dataFilesToAdd, deleteFilesToRemove []iceberg.DataFile, snapshotProps
iceberg.Properties, opts ...WriteOption) error {
- return t.replaceFiles(ctx, dataFilesToDelete, dataFilesToAdd,
deleteFilesToRemove, nil, nil, snapshotProps, opts...)
-}
-
-// ReplaceFilesWithDeleteFiles atomically replaces data files, adds new delete
-// files, and removes superseded delete files in one snapshot. Every added
delete
-// file must carry the data sequence number of the delete files it replaces.
-// This is intended for rewrites that change delete-file representation, such
as
-// rewriting position deletes into deletion vectors.
-func (t *Transaction) ReplaceFilesWithDeleteFiles(ctx context.Context,
dataFilesToDelete, dataFilesToAdd, deleteFilesToRemove []iceberg.DataFile,
deleteFilesToAdd []DeleteFileAddition, snapshotProps iceberg.Properties, opts
...WriteOption) error {
+// ReplaceFiles atomically replaces data files, adds new delete files, and
+// removes superseded delete files in one snapshot. For compaction, old data
+// files are replaced with compacted ones and fully applied deletes are
removed.
+// On v3, a removed data file's live deletion vector is removed with it even if
+// it is not in deleteFilesToRemove.
+// Every added delete file must carry the data sequence number of the delete
+// files it replaces. The method also supports rewrites that change delete-file
+// representation, such as rewriting position deletes into deletion vectors.
+// Direct calls record an overwrite snapshot with overwrite validation. Use
+// [Transaction.NewRewrite] when a rewrite preserves table rows and should
+// record a replace snapshot instead.
+func (t *Transaction) ReplaceFiles(ctx context.Context, dataFilesToDelete,
dataFilesToAdd, deleteFilesToRemove []iceberg.DataFile, deleteFilesToAdd
[]DeleteFileAddition, snapshotProps iceberg.Properties, opts ...WriteOption)
error {
Review Comment:
Three adjacent `[]iceberg.DataFile` parameters plus `deleteFilesToAdd` can
be swapped without a compile error, which is the shape the builder exists to
avoid. Could this keep its shipped 5-argument signature, with delete-file
additions going through `NewRewrite().AddDeleteFileWithDataSequenceNumber(...)`?
--
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]
