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 (`latestSnapshotId`)
to the whole merged row. If snapshot 1 writes `b=100`, snapshot 2 writes
`b=200` and does not participate in this compaction, and snapshot 3 writes
`c=300` and is compacted together with snapshot 1, the compacted row may carry
snapshotId=3 for both fields and later override snapshot 2's `b=200` with
snapshot 1's old `b=100`. Could you add a `partial-update` 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]
