vinishjail97 commented on code in PR #951:
URL: https://github.com/apache/incubator-xtable/pull/951#discussion_r4169460972
##########
xtable-core/src/test/java/org/apache/xtable/hudi/TestHudiFileStatsExtractor.java:
##########
@@ -332,6 +332,77 @@ void
columnStatsWithMetadataTablePartialMissingFallsBackToParquetFooters(@TempDi
verify(mockMetaClient, times(1)).getStorage();
}
+ @Test
+ void
columnStatsWithMetadataTableZeroValueCountFallsBackToParquetFooters(@TempDir
Path tempDir) {
+ // Reproduces a real-world case: the metadata table has an entry for the
file (so it isn't
+ // "missing"), but every column's value count is 0 -- e.g. for files
written before
+ // column-stats indexing was enabled for the table. Previously this was
accepted as a
+ // successful enrichment with recordCount=0 (silently wrong for a
non-empty file); it should
+ // instead be treated like "no stats" and fall back to the parquet footer.
+ List<InternalDataFile> inputFiles = generateInputFiles(tempDir, 1);
+ InternalDataFile file = inputFiles.get(0);
+ Pair<String, String> filePair =
+ Pair.of("", new
org.apache.hadoop.fs.Path(file.getPhysicalPath()).getName());
+
+ HoodieTableConfig mockTableConfig = mock(HoodieTableConfig.class);
+
when(mockTableConfig.isMetadataPartitionAvailable(MetadataPartitionType.COLUMN_STATS))
+ .thenReturn(true);
+ when(mockTableConfig.getTableVersion()).thenReturn(HoodieTableVersion.SIX);
+
+ HoodieTableMetaClient mockMetaClient = mock(HoodieTableMetaClient.class);
+ doReturn(storageConf).when(mockMetaClient).getStorageConf();
+ doReturn(new HoodieHadoopStorage(new
StoragePath(tempDir.toUri().getPath()), storageConf))
+ .when(mockMetaClient)
+ .getStorage();
+ when(mockMetaClient.getIndexMetadata()).thenReturn(Option.empty());
+ when(mockMetaClient.getBasePath()).thenReturn(new
StoragePath(tempDir.toUri().getPath()));
+ when(mockMetaClient.getTableConfig()).thenReturn(mockTableConfig);
+
+ HoodieTableMetadata mockMetadataTable = mock(HoodieTableMetadata.class);
+ when(mockMetadataTable.getColumnStats(any(), anyString()))
+ .thenAnswer(
+ invocation -> {
+ String fieldName = invocation.getArgument(1);
+ Map<Pair<String, String>, HoodieMetadataColumnStats> statsMap =
new HashMap<>();
+ statsMap.put(
+ filePair,
+ HoodieMetadataColumnStats.newBuilder()
+ .setFileName(filePair.getRight())
+ .setColumnName(fieldName)
+ .setValueCount(0L)
+ .setNullCount(0L)
+ .setTotalSize(0L)
+ .setIsDeleted(false)
+ .build());
+ return statsMap;
+ });
+
+ HudiFileStatsExtractor extractor = new
HudiFileStatsExtractor(mockMetaClient);
+ List<InternalDataFile> output =
+ extractor
+ .addStatsToFiles(mockMetadataTable, inputFiles.stream(), schema)
+ .collect(Collectors.toList());
+
+ assertEquals(1, output.size());
+ InternalDataFile result = output.get(0);
Review Comment:
Would it work to end this test with `validateOutput(output, false);`, like
`columnStatsWithMetadataTableMissingFallsBackToParquetFooters` does? It checks
the record count and every footer column stat, so the hand-written assertions
could go. I tried it locally and it passes. Non-blocking, and the shared mock
setup across the three fallback tests could become a helper as a follow-up.
--
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]
