zhang-arvin commented on PR #9363: URL: https://github.com/apache/paimon/pull/9363#issuecomment-5469136347
@JingsongLi Thanks for the design suggestion! Passing a single Snapshot object instead of (snapshotId, snapshotUuid) is a cleaner API — I agree it reduces the risk of mismatched pairs and handles legacy null-UUID snapshots through equality. However, implementing this change requires: 1. Adding a nullable Snapshot field to ConflictDetection (the base class held by FileStoreCommitImpl) 2. Updating all callers (BatchWriteBuilderImpl, DataEvolutionMergeIntoAction, DataEvolutionDeleteSink, Spark MERGE) to pass the Snapshot 3. The lineage validation before empty RowIdConflictChecker return This is a larger refactoring than the current approach. Would you prefer I implement this change in this PR, or can we land the current fix first (which already passes CI and covers the ABA scenario) and follow up with the API improvement in a separate PR? The current approach already: - Detects rollback when latestSnapshot.id() < baseSnapshotId - Detects ABA when same snapshot ID has different UUID - Handles null baseSnapshot gracefully - Has regression tests in ConflictDetectionTest -- 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]
