laskoviymishka commented on code in PR #947:
URL: https://github.com/apache/iceberg-go/pull/947#discussion_r3167095929
##########
table/rewrite_data_files.go:
##########
@@ -63,24 +71,54 @@ type CompactionTaskGroup struct {
TotalSizeBytes int64
}
-// RewriteDataFiles compacts the given groups by reading data with deletes
-// applied, writing new consolidated files, and atomically replacing the
-// old files. Position delete files that are fully applied are removed.
-//
-// Use table/compaction.Config.PlanCompaction() to produce the groups,
-// then convert compaction.Group → CompactionTaskGroup and pass them here.
+// RewriteDataFilesOptions bundles the per-rewrite knobs for
+// Transaction.RewriteDataFiles.
+type RewriteDataFilesOptions struct {
+ // PartialProgress, when true, stages each group via ReplaceFiles
+ // inside the loop so work survives a mid-loop write failure. When
+ // false (the default), all groups are committed in a single atomic
+ // snapshot.
+ //
+ // In both modes the final catalog commit happens once at
+ // Transaction.Commit() time. True per-group durability (matching
+ // Java's behavior) requires committing separate transactions per
+ // group, which is left to the caller.
+ PartialProgress bool
+
+ // SnapshotProps are added to the rewrite snapshot's summary.
+ SnapshotProps iceberg.Properties
+
+ // ExtraDeleteFilesToRemove are delete files (typically equality
+ // deletes that are dead after the rewrite) that the caller wants
+ // expunged in the same snapshot as the rewrite. The executor
+ // passes them through to ReplaceFiles unchanged. Honored only
+ // when PartialProgress is false.
+ //
+ // Use table/compaction.CollectDeadEqualityDeletes to compute this
+ // list from the current snapshot. Position delete files that are
+ // fully applied are removed automatically and do NOT need to be
+ // passed in here.
+ ExtraDeleteFilesToRemove []iceberg.DataFile
+}
+
+// RewriteDataFiles compacts the given groups by reading data with
+// deletes applied, writing new consolidated files, and atomically
+// replacing the old files. Position delete files that are fully
+// applied (every referenced data file is in the rewrite set) are
+// removed automatically.
//
-// Note: partialProgress accumulates all group commits within this
-// transaction. True per-group durability (matching Java's behavior)
-// requires committing separate transactions per group, which is left
-// to the caller. When partialProgress is false, all groups are committed
-// in a single atomic snapshot.
+// Equality-delete cleanup is the caller's responsibility: compute the
+// dead set with table/compaction.CollectDeadEqualityDeletes (against
+// the same snapshot the rewrite is staged on) and pass it via
+// opts.ExtraDeleteFilesToRemove. The executor only orchestrates the
+// commit; it does not impose a cleanup policy. This split keeps the
+// pure spec predicate in table/compaction and the unexported snapshot
+// machinery in table.
Review Comment:
fixed everywhere
--
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]
