the-other-tim-brown commented on code in PR #13444:
URL: https://github.com/apache/hudi/pull/13444#discussion_r2191402190
##########
hudi-common/src/main/java/org/apache/hudi/common/table/read/FileGroupRecordBuffer.java:
##########
@@ -565,27 +570,62 @@ protected boolean hasNextBaseRecord(T baseRecord,
BufferedRecord<T> logRecordInf
Pair<Boolean, T> isDeleteAndRecord = merge(baseRecordInfo,
logRecordInfo);
if (!isDeleteAndRecord.getLeft()) {
// Updates
- nextRecord = readerContext.seal(isDeleteAndRecord.getRight());
+ nextRecord =
readerContext.seal(applyOutputSchemaConversion(isDeleteAndRecord.getRight()));
Review Comment:
Don't forget `SortedKeyBasedFileGroupRecordBuffer`. We already need 3 right
now, but what if more merging strategies are added? That is what I mean by
maintenance.
Subclassing is not cleaner. There is even a famous OOP principle about this
that I am sure you know.
If we really care about a single check that happens for a subset of rows,
why isn't the same scrutiny applied to other PRs regardless of whether it is
new logic or not? The code changed even if the core logic is preserved. It is
an opportunity for improvement.
--
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]
