yihua commented on code in PR #12122:
URL: https://github.com/apache/hudi/pull/12122#discussion_r1819815014
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergeHandle.java:
##########
@@ -355,22 +357,28 @@ public void write(HoodieRecord<T> oldRecord) {
// writing the first record. So make a copy of the record to be merged
HoodieRecord<T> newRecord = keyToNewRecords.get(key).newInstance();
try {
- Option<Pair<HoodieRecord, Schema>> mergeResult =
recordMerger.merge(oldRecord, oldSchema, newRecord, newSchema, props);
- Schema combineRecordSchema =
mergeResult.map(Pair::getRight).orElse(null);
- Option<HoodieRecord> combinedRecord = mergeResult.map(Pair::getLeft);
- if (combinedRecord.isPresent() &&
combinedRecord.get().shouldIgnore(combineRecordSchema, props)) {
- // If it is an IGNORE_RECORD, just copy the old record, and do not
update the new record.
- copyOldRecord = true;
- } else if (writeUpdateRecord(newRecord, oldRecord, combinedRecord,
combineRecordSchema)) {
- /*
- * ONLY WHEN 1) we have an update for this key AND 2) We are able to
successfully
- * write the combined new value
- *
- * We no longer need to copy the old record over.
- */
- copyOldRecord = false;
+ if
(keyToNewRecords.get(key).getMetaDataInfo(DELETE_FOUND_WITHOUT_ORDERING_VALUE).isPresent())
{
+ if (writeUpdateRecord(newRecord, oldRecord, Option.empty(),
newSchema)) {
+ copyOldRecord = false;
+ }
+ } else {
+ Option<Pair<HoodieRecord, Schema>> mergeResult =
recordMerger.merge(oldRecord, oldSchema, newRecord, newSchema, props);
Review Comment:
Could the `DELETE_FOUND_WITHOUT_ORDERING_VALUE` handling be incorporated
into the record merger implementation? The write handle should not contain any
such specific handling on merging behavior.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergeHandle.java:
##########
@@ -355,22 +357,28 @@ public void write(HoodieRecord<T> oldRecord) {
// writing the first record. So make a copy of the record to be merged
HoodieRecord<T> newRecord = keyToNewRecords.get(key).newInstance();
try {
- Option<Pair<HoodieRecord, Schema>> mergeResult =
recordMerger.merge(oldRecord, oldSchema, newRecord, newSchema, props);
- Schema combineRecordSchema =
mergeResult.map(Pair::getRight).orElse(null);
- Option<HoodieRecord> combinedRecord = mergeResult.map(Pair::getLeft);
- if (combinedRecord.isPresent() &&
combinedRecord.get().shouldIgnore(combineRecordSchema, props)) {
- // If it is an IGNORE_RECORD, just copy the old record, and do not
update the new record.
- copyOldRecord = true;
- } else if (writeUpdateRecord(newRecord, oldRecord, combinedRecord,
combineRecordSchema)) {
- /*
- * ONLY WHEN 1) we have an update for this key AND 2) We are able to
successfully
- * write the combined new value
- *
- * We no longer need to copy the old record over.
- */
- copyOldRecord = false;
+ if
(keyToNewRecords.get(key).getMetaDataInfo(DELETE_FOUND_WITHOUT_ORDERING_VALUE).isPresent())
{
Review Comment:
Use `newRecord` instead `keyToNewRecords.get(key)`?
--
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]
