danny0405 commented on code in PR #12971:
URL: https://github.com/apache/hudi/pull/12971#discussion_r1993030977
##########
hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieBaseFileGroupRecordBuffer.java:
##########
@@ -322,11 +322,13 @@ protected Option<DeleteRecord>
doProcessNextDeletedRecord(DeleteRecord deleteRec
return Option.empty();
}
Comparable deleteOrderingVal = deleteRecord.getOrderingValue();
- // Checks the ordering value does not equal to 0
- // because we use 0 as the default value which means natural order
- boolean chooseExisting = !deleteOrderingVal.equals(0)
- && ReflectionUtils.isSameClass(existingOrderingVal,
deleteOrderingVal)
- && existingOrderingVal.compareTo(deleteOrderingVal) > 0;
+
+ // Because the delete record from older log block, so we should deal
it with three cases:
+ // 1. delete record with invalid ordering value: keep existing
record because it is newer, deletion can be ignored.
+ // 2. delete record with valid ordering value and higher than
existing record: keep delete record because it is newer.
+ // 3. delete record with valid ordering value and lower than
existing record: keep existing record because it is newer, deletion can be
ignored.
+ boolean chooseExisting = deleteOrderingVal.equals(0)
Review Comment:
this is still not correct, because the delete block lost the processing time
sequence semantics, cc @yihua , how do we handle the deletes with processing
time sequence semantics now? It looks like we can not cover this scenario
because the delete block lost the info of processing time sequence(a delete
paylod always has higher priority than the data block payload in this
processing time merging scenario).
--
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: commits-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
