yihua commented on code in PR #13008:
URL: https://github.com/apache/hudi/pull/13008#discussion_r2015170723
##########
hudi-common/src/test/java/org/apache/hudi/common/table/read/TestHoodieFileGroupReaderBase.java:
##########
@@ -148,6 +157,34 @@ public void
testReadFileGroupInMergeOnReadTable(RecordMergeMode recordMergeMode,
}
}
+ @ParameterizedTest
+ @EnumSource(RecordMergeMode.class)
+ public void testFileGroupReaderBasedCompaction(RecordMergeMode mergeMode)
throws Exception {
Review Comment:
The file group reader exposes `readStats`. Could you validate that as part
of existing FG reader tests?
##########
hudi-common/src/test/java/org/apache/hudi/common/table/read/TestHoodieFileGroupReaderBase.java:
##########
@@ -250,6 +287,42 @@ private Map<String, String>
getCommonConfigs(RecordMergeMode recordMergeMode) {
return configMapping;
}
+ private void validateCompactionStats(StorageConfiguration<?> storageConf,
Review Comment:
We should not validate the compaction stats at the test in this layer. It
should be validated in compaction functional tests.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/read/FileGroupRecordBuffer.java:
##########
@@ -238,6 +243,7 @@ protected Option<Pair<Option<T>, Map<String, Object>>>
doProcessNextDataRecord(T
Map<String, Object> metadata,
Pair<Option<T>, Map<String, Object>> existingRecordMetadataPair)
throws IOException {
+ totalLogRecords++;
Review Comment:
Why do we need this? The file group reader already does
`readStats.setTotalLogRecords(logRecordReader.getTotalLogRecords())`.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/read/FileGroupRecordBuffer.java:
##########
@@ -238,6 +243,7 @@ protected Option<Pair<Option<T>, Map<String, Object>>>
doProcessNextDataRecord(T
Map<String, Object> metadata,
Pair<Option<T>, Map<String, Object>> existingRecordMetadataPair)
throws IOException {
+ totalLogRecords++;
Review Comment:
Should the counter be incremented inside the log reader instead?
--
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
