danny0405 commented on code in PR #6697:
URL: https://github.com/apache/hudi/pull/6697#discussion_r977089707
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieSortedMergeHandle.java:
##########
@@ -93,13 +94,18 @@ public void write(GenericRecord oldRecord) {
throw new HoodieUpsertException("Insert/Update not in sorted order");
}
try {
+ Option<IndexedRecord> insertRecord;
if (useWriterSchemaForCompaction) {
- writeRecord(hoodieRecord,
hoodieRecord.getData().getInsertValue(tableSchemaWithMetaFields,
config.getProps()));
+ insertRecord =
hoodieRecord.getData().getInsertValue(tableSchemaWithMetaFields,
config.getProps());
} else {
- writeRecord(hoodieRecord,
hoodieRecord.getData().getInsertValue(tableSchema, config.getProps()));
+ insertRecord = hoodieRecord.getData().getInsertValue(tableSchema,
config.getProps());
}
+ writeRecord(hoodieRecord, insertRecord);
insertRecordsWritten++;
writtenRecordKeys.add(keyToPreWrite);
+ if (cdcEnabled) {
+ cdcLogger.put(hoodieRecord, null, insertRecord);
+ }
Review Comment:
If sorted merge deserves a sub-class, we should follow that and give cdc
feature a sub-class too, I would see it as a prove that we should keep good
expansibility for different use cases and components.
> it will quickly become unmanageable
Why you call it unmanageable if we only add 2 classes here ? I didn't feel
that based on the fact that I already added 5 handles for flink. We can manage
them because they are instantiated in a factory in the base commit executor.
BTW, Imagine what a mess the code is if i put these flink logic into the
base handles. But i agree we need some refactoring to the base handles but that
should be very small.
--
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]
