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]

Reply via email to