hudi-agent commented on code in PR #19347:
URL: https://github.com/apache/hudi/pull/19347#discussion_r3633040218
##########
hudi-common/src/test/java/org/apache/hudi/common/table/read/lsm/TestLsmFileGroupRecordIterator.java:
##########
@@ -119,6 +151,95 @@ void
testSelectDirectLogReadersPrioritizesDeletesThenSmallFiles() {
assertEquals(new HashSet<>(Arrays.asList(2, 3)), directReadersWithoutBase);
}
+ @Test
+ void testSortedRunFixturesMergeAcrossBaseAndSpilledLogs() throws IOException
{
+ HoodieTableConfig tableConfig = new HoodieTableConfig();
+ tableConfig.setValue(HoodieTableConfig.RECORDKEY_FIELDS, "id");
+ tableConfig.setValue(HoodieTableConfig.ORDERING_FIELDS, "ts");
+ tableConfig.setValue(HoodieTableConfig.RECORD_MERGE_MODE,
RecordMergeMode.EVENT_TIME_ORDERING.name());
+ tableConfig.setValue(HoodieTableConfig.BASE_FILE_FORMAT,
HoodieFileFormat.PARQUET.name());
+ tableConfig.setValue(HoodieTableConfig.POPULATE_META_FIELDS, "false");
+ StorageConfiguration<?> storageConfiguration =
mock(StorageConfiguration.class);
+ HoodieAvroReaderContext context = org.mockito.Mockito.spy(
Review Comment:
🤖 nit: `org.mockito.Mockito.spy(...)` stands out here — `doAnswer`, `mock`,
and `when` are all statically imported in this file, so adding `import static
org.mockito.Mockito.spy;` and using `spy(...)` directly would be more
consistent.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/test/java/org/apache/hudi/common/table/read/lsm/TestLsmFileGroupRecordIterator.java:
##########
@@ -119,6 +151,95 @@ void
testSelectDirectLogReadersPrioritizesDeletesThenSmallFiles() {
assertEquals(new HashSet<>(Arrays.asList(2, 3)), directReadersWithoutBase);
}
+ @Test
+ void testSortedRunFixturesMergeAcrossBaseAndSpilledLogs() throws IOException
{
+ HoodieTableConfig tableConfig = new HoodieTableConfig();
+ tableConfig.setValue(HoodieTableConfig.RECORDKEY_FIELDS, "id");
+ tableConfig.setValue(HoodieTableConfig.ORDERING_FIELDS, "ts");
+ tableConfig.setValue(HoodieTableConfig.RECORD_MERGE_MODE,
RecordMergeMode.EVENT_TIME_ORDERING.name());
+ tableConfig.setValue(HoodieTableConfig.BASE_FILE_FORMAT,
HoodieFileFormat.PARQUET.name());
+ tableConfig.setValue(HoodieTableConfig.POPULATE_META_FIELDS, "false");
+ StorageConfiguration<?> storageConfiguration =
mock(StorageConfiguration.class);
+ HoodieAvroReaderContext context = org.mockito.Mockito.spy(
+ new HoodieAvroReaderContext(storageConfiguration, tableConfig,
Option.empty(), Option.empty()));
+ TypedProperties props = new TypedProperties();
+ props.setProperty(HoodieReaderConfig.LSM_SORT_MERGE_SPILL_THRESHOLD.key(),
"1");
+ props.setProperty(HoodieMemoryConfig.SPILLABLE_MAP_BASE_PATH.key(),
tempDir.toString());
+ context.initRecordMerger(props);
+
+ FileGroupReaderSchemaHandler<IndexedRecord> schemaHandler =
mock(FileGroupReaderSchemaHandler.class);
+ when(schemaHandler.getRequiredSchema()).thenReturn(tableSchema());
+ when(schemaHandler.getRequestedSchema()).thenReturn(tableSchema());
+ when(schemaHandler.getTableSchema()).thenReturn(tableSchema());
+ when(schemaHandler.getDeleteContext()).thenReturn(new DeleteContext(props,
tableSchema()));
+
when(schemaHandler.getRequiredSchemaForFileAndRenamedColumns(any(StoragePath.class)))
+ .thenReturn(Pair.of(tableSchema(), Collections.emptyMap()));
+ context.setSchemaHandler(schemaHandler);
+ context.setTablePath("/tmp/lsm-fixture");
+
+ doAnswer(invocation -> {
+ StoragePathInfo pathInfo = invocation.getArgument(0);
+ String name = pathInfo.getPath().getName();
+ if (name.endsWith("deletes.parquet")) {
+ HoodieSchema deleteSchema = invocation.getArgument(3);
+ return
ClosableIterator.wrap(Collections.singletonList(deleteRecord(deleteSchema, "b",
2L)).iterator());
+ }
+ List<IndexedRecord> records;
+ if (name.endsWith(".parquet") && !name.contains(".log.")) {
+ records = Arrays.asList(indexedRecord("a", "base-a", 1),
indexedRecord("c", "base-c", 5));
+ } else if (name.contains("_002_")) {
+ records = Arrays.asList(indexedRecord("a", "log2-a", 3),
indexedRecord("c", "stale-c", 2));
+ } else {
+ records = Arrays.asList(indexedRecord("a", "log1-a", 2),
indexedRecord("b", "log1-b", 1));
+ }
+ return ClosableIterator.wrap(records.iterator());
+ }).when(context).getFileRecordIterator(any(StoragePathInfo.class),
anyLong(), anyLong(),
+ any(HoodieSchema.class), any(HoodieSchema.class),
any(HoodieStorage.class));
+
+ HoodieBaseFile baseFile = new
HoodieBaseFile(pathInfo("/tmp/file1_1-0-1_001.parquet", 100));
+ List<HoodieLogFile> logs = Arrays.asList(
+ new HoodieLogFile(pathInfo("/tmp/file1_1-0-1_001_1.log.parquet", 50)),
+ new HoodieLogFile(pathInfo("/tmp/file1_1-0-1_002_1.log.parquet", 60)),
+ new HoodieLogFile(pathInfo("/tmp/file1_1-0-1_003_1.deletes.parquet",
10)));
+ InputSplit split = InputSplit.builder()
+ .baseFileOption(Option.of(baseFile))
+ .logFileStream(logs.stream())
+ .partitionPath("")
+ .start(0)
+ .length(Long.MAX_VALUE)
+ .build();
+ HoodieTableMetaClient metaClient = mock(HoodieTableMetaClient.class);
+ when(metaClient.getTableConfig()).thenReturn(tableConfig);
+ ReaderParameters parameters =
ReaderParameters.builder().emitDeletes(false).build();
+
+ List<String> actual = new ArrayList<>();
+ try (LsmFileGroupRecordIterator<IndexedRecord> iterator = new
LsmFileGroupRecordIterator<>(
+ context, mock(HoodieStorage.class), split,
Collections.singletonList("ts"), metaClient,
+ props, parameters, new HoodieReadStats(), Option.empty())) {
+ assertTrue(iterator.hasNext());
Review Comment:
🤖 nit: the two consecutive `assertTrue(iterator.hasNext())` calls here look
like a copy-paste mistake at first glance — if this is intentionally testing
that `hasNext()` is idempotent, a short comment like `// verify hasNext() is
idempotent` would make the intent clear.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
