JingsongLi commented on code in PR #7873:
URL: https://github.com/apache/paimon/pull/7873#discussion_r3458872721


##########
paimon-python/pypaimon/write/file_store_commit.py:
##########
@@ -189,6 +191,98 @@ def commit(self, commit_messages: List[CommitMessage], 
commit_identifier: int):
                          allow_rollback=allow_rollback,
                          index_deletes=index_deletes)
 
+    def commit_compact(self, commit_messages: List[CommitMessage], 
commit_identifier: int):
+        """Commit compaction-only messages.
+
+        Each message must carry no new_files; compact_before → DELETE entries,
+        compact_after → ADD entries; snapshot kind = COMPACT. Skips row-id
+        assignment and conflict detection (a compaction never produces new 
rows).
+        """
+        if not commit_messages:
+            return
+
+        for msg in commit_messages:
+            if msg.new_files:
+                raise ValueError(
+                    "commit_compact rejects messages with new_files; use 
commit() instead."
+                )
+
+        logger.info(
+            "Ready to commit compact to table %s, number of commit messages: 
%d",
+            self.table.identifier,
+            len(commit_messages),
+        )
+        commit_entries = self._build_commit_entries(commit_messages)
+        if not commit_entries:
+            return
+
+        logger.info("Finished collecting compact changes: %d entries", 
len(commit_entries))
+
+        self._try_commit(

Review Comment:
   `commit_compact()` drops `CommitMessage.index_deletes`. The normal 
`commit()` path collects these deletions and passes them into `_try_commit`, 
and the serializer now explicitly preserves `index_deletes` because a message 
can be non-empty only due to index manifest deletions. Here, however, compact 
messages are neither rejected when `msg.index_deletes` is set nor forwarded to 
`_try_commit`, so any compaction task that emits index manifest deletions would 
silently leave the old index manifest entries behind. Could we either 
collect/pass `index_deletes` here like `commit()` does, or explicitly reject 
them until compact-side index deletion is wired, with a regression test?



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

Reply via email to