leaves12138 commented on code in PR #7832:
URL: https://github.com/apache/paimon/pull/7832#discussion_r3257674310
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/PartialUpdateMergeFunction.java:
##########
@@ -358,7 +362,8 @@ public KeyValue getResult() {
}
RowKind rowKind = currentDeleteRow || !meetInsert ? RowKind.DELETE :
RowKind.INSERT;
- return reused.replace(currentKey, latestSequenceNumber, rowKind, row);
+ return reused.replace(currentKey, latestSequenceNumber, rowKind, row)
Review Comment:
Thanks for fixing the missing snapshotId propagation. One more
partial-update edge case may need either an explanation or a regression test: a
compacted partial-update result can contain fields coming from different input
snapshots, but this code assigns one row-level snapshotId () to the whole
merged row. If snapshot 1 writes , snapshot 2 writes and does not participate
in this compaction, and snapshot 3 writes and is compacted together with
snapshot 1, the compacted row may carry snapshotId=3 for both fields and later
override snapshot 2's with snapshot 1's old . Could you add a version of the
non-contiguous compaction test (similar to the deduplicate ordering-reversal
case) to prove this cannot happen?
--
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]