leaves12138 commented on code in PR #8915:
URL: https://github.com/apache/paimon/pull/8915#discussion_r3680294857
##########
paimon-python/pypaimon/tests/table_upsert_by_key_test.py:
##########
@@ -359,6 +399,227 @@ def test_upsert_across_multiple_data_files(self):
self.assertEqual('Dave', rows[4])
self.assertEqual('Eve', rows[5])
+ def test_commit_rewrites_stale_update_after_compaction(self):
+ table = self._create_table()
+ self._write_arrow(table, pa.Table.from_pydict({
+ 'id': [1, 2],
+ 'name': ['Alice', 'Bob'],
+ 'age': [25, 30],
+ 'city': ['NYC', 'LA'],
+ }, schema=self.pa_schema))
+ self._write_arrow(table, pa.Table.from_pydict({
+ 'id': [3, 4],
+ 'name': ['Carol', 'Dave'],
+ 'age': [35, 40],
+ 'city': ['Chicago', 'Houston'],
+ }, schema=self.pa_schema))
+
+ wb = self._make_write_builder(table)
+ update = wb.new_update().with_update_type(['age', 'city'])
+ commit_identifier = self._next_commit_id()
+ messages = self._apply_upsert(
+ update,
+ pa.Table.from_pydict({
+ 'id': [2, 3],
+ 'name': ['ignored', 'ignored'],
+ 'age': [31, 36],
+ 'city': ['LA2', 'Chicago2'],
+ }, schema=self.pa_schema),
+ ['id'],
+ commit_identifier,
+ )
+ stale_paths = [
+ file.file_path
+ for message in messages
+ for file in message.new_files
+ ]
+
+ self._compact_all_data_files(table)
+
+ commit = wb.new_commit()
+ self._apply_commit(commit, messages, commit_identifier)
+ commit.close()
+
+ rows = {
+ row['id']: (row['name'], row['age'], row['city'])
+ for row in self._read_all(table).to_pylist()
+ }
+ self.assertEqual(('Bob', 31, 'LA2'), rows[2])
+ self.assertEqual(('Carol', 36, 'Chicago2'), rows[3])
+ self.assertEqual(('Dave', 40, 'Houston'), rows[4])
+ self.assertTrue(all(os.path.exists(path) for path in stale_paths))
Review Comment:
This assertion codifies an orphan-file leak. `RowIdConflictRewriter.rewrite`
removes these candidate entries from the commit and returns only the
replacement entries, but neither the rewriter nor `_try_commit` deletes the
discarded files after the rewritten commit succeeds. `_abort` only cleans newly
generated files when rewrite generation itself fails. As a result, every
recovered conflict leaves the original partial-update files unreferenced by any
snapshot; if another compaction causes a second rewrite, the intermediate
generation is leaked as well, and retry exhaustion after a `RewriteResult` also
leaves generated files behind. Please carry file ownership through the
rewrite/retry loop, clean all deterministically discarded generations, and
change this regression assertion to verify that the stale paths are removed
after a successful replacement commit.
--
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]
