JingsongLi commented on PR #966: URL: https://github.com/apache/paimon-rust/pull/966#issuecomment-5935395161
Requirement fit: SUPPORTED. Implementation: FINDINGS. [P1] Protect branch references before automatic expiration (`crates/paimon/src/table/table_commit.rs:383`, shared deletion in `expire_snapshots.rs:222-260`). This turns the branch-reference issue in #965 into a normal commit side effect. Reproduced at this head: configure snapshot min/max=1; write row 1; create tag t1 and branch b1 from snapshot 1; delete the main tag; verify b1 reads row 1; overwrite main with row 2. The overwrite succeeds and main reads row 2, but b1 now fails with NotFound for its original manifest list. No explicit expiration call is involved. Protect every live branch owner before deleting shared data/manifests, or skip automatic expiration until that protection is implemented. See also https://github.com/apache/paimon-rust/pull/968#issuecomment-5935241081. [P2] Run maintenance on successful ignored empty commits (`crates/paimon/src/table/table_commit.rs:383`; earlier return at 363-365). The PR explicitly says maintenance does not depend on creating a snapshot, matching expireForEmptyCommit. However commit(Vec::new()) with the default ignore_empty_commit returns before the new maintain call. A real test creates three snapshots with write-only=true and retain min/max=1, enables maintenance, and successfully commits no messages: all [1,2,3] remain instead of [3]. Existing data remains correct, but idle successful batches cannot reclaim expired history as promised. Handle this successful no-snapshot path too, and add an empty-commit retention test. This is a verified gap in the new maintenance requirement, not a newly introduced empty-commit behavior. The shared manual expiration API also retains the persisted-changelog safety defect described in https://github.com/apache/paimon-rust/pull/967#issuecomment-5935349402. The new automatic decoupled-lifecycle skip is appropriate and was not conflated with that manual path. Validation at 13392b6f14949ac954690f8f9a0fd42e63c4a4c2: all 29 expiration/automatic-maintenance tests and 85 core-option tests passed; both additional behavioral probes above fail. Commit success/error/skip paths were independently reviewed. Current main conflicts in table/mod.rs, so rebase and rerun before merge. -- 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]
