vamsikarnika commented on code in PR #951:
URL: https://github.com/apache/incubator-xtable/pull/951#discussion_r4120076223
##########
xtable-core/src/test/java/org/apache/xtable/hudi/TestHudiFileStatsExtractor.java:
##########
@@ -332,6 +332,66 @@ 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);
+ // Must have fallen back to the parquet footer (real row count), not the
degenerate
+ // metadata-table stats (which would previously have produced
recordCount=0).
+ assertEquals(2, result.getRecordCount());
+ assertFalse(result.getColumnStats().isEmpty());
Review Comment:
Makes sense. Updated the test to add more checks.
--
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]
