nsivabalan commented on code in PR #13314:
URL: https://github.com/apache/hudi/pull/13314#discussion_r2094703531
##########
hudi-common/src/test/java/org/apache/hudi/common/table/read/TestHoodieFileGroupReaderBase.java:
##########
@@ -317,6 +319,13 @@ private void
validateOutputFromFileGroupReaderWithExistingRecords(StorageConfigu
// validate size is equivalent to ensure no duplicates are returned
assertEquals(expectedRecords.size(), actualRecordList.size());
assertEquals(new HashSet<>(expectedRecords), new
HashSet<>(actualRecordList));
+ // validate records can be read from file group as HoodieRecords
+ actualRecordList = convertHoodieRecords(
+ readHoodieRecordsFromFileGroup(storageConf, tablePath, metaClient,
fileSlices, avroSchema, recordMergeMode),
+ avroSchema, readerContext);
+ assertEquals(expectedRecords.size(), actualRecordList.size());
Review Comment:
not related to this patch. but I was reviewing this test class in general.
looks like to compute final expected records (merging inserts + updates), we
just choose the later if a record is part of update list (I am referring to
method named "mergeRecordLists". So, this means, we can have tests only on
commit time based merge mode. for others, the way we compute expected list of
records may not work unless we fix this method impl.
not something we need to fix anything in this patch.but just wanted to call
out that I noticed.
CC @yihua
--
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]
