yihua commented on code in PR #12971:
URL: https://github.com/apache/hudi/pull/12971#discussion_r1993525851
##########
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:
@linliu-code could you link you PR?
##########
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:
I discussed this issue with @linliu-code and he has a fix.
--
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
