This is an automated email from the ASF dual-hosted git repository.
the-other-tim-brown pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-xtable.git
The following commit(s) were added to refs/heads/main by this push:
new 09509183 fix(hudi): Fall back to parquet-footer stats when the
metadata table returns degenerate value counts (#951)
09509183 is described below
commit 0950918368c6f6235d288e3bfad89db1acb442cf
Author: vamsikarnika <[email protected]>
AuthorDate: Wed Oct 7 07:03:21 2026 +0530
fix(hudi): Fall back to parquet-footer stats when the metadata table
returns degenerate value counts (#951)
* fix(hudi): Fall back to parquet-footer stats when the metadata table
returns degenerate value counts
tryEnrichWithMetadataStats() accepted the Hudi metadata table's
column-stats entry for a file as valid whenever the entry existed at
all, even when every column's value count came back non-positive (or
absent). In that case getMaxFromColumnStats() falls through to
.orElse(0L), silently emitting recordCount=0 for a file that has real
data -- the existing fallback to parquet-footer stats only triggers when
the metadata table has no entry for the file at all, not when it has
an entry with unusable values.
Observed in practice on files written before column-stats indexing was
enabled for their table: real Parquet files with correct row counts in
their own footers, but zero-valued entries in the metadata table's
column-stats index, ended up with recordCount=0 (and empty column
stats) in the synced Iceberg table's manifest -- real data, wrong
per-file statistics.
Fix: treat an absent/non-positive max the same as "no stats" and fall
back to reading the file's actual row count from its Parquet footer,
reusing the existing (already correct) fallback path.
Adds a regression test reproducing the exact scenario: metadata table
returns a non-empty entry with valueCount=0 for every column; asserts
the result falls back to the parquet footer (real row count) instead
of emitting recordCount=0.
* Address review feedback from @vinishjail97 and @the-other-tim-brown
- Reword the log.warn in addStatsToFiles() to say files had 'missing or
unusable' column stats, since it now covers both the absent-entry case
and the degenerate-zero-value-count case, not just the former.
- Simplify tryEnrichWithMetadataStats's if/return into a single
getMaxFromColumnStats(...).map(...) expression.
- Strengthen the new regression test's assertion: instead of only
checking columnStats isn't empty (which degenerate zero-count stats
would also satisfy if they leaked through), assert the actual
footer-derived values for a specific column (numValues, numNulls,
min/max range).
* simplify test setup, reuse code
---------
Co-authored-by: Tim Brown <[email protected]>
---
.../apache/xtable/hudi/HudiFileStatsExtractor.java | 12 ++-
.../xtable/hudi/TestHudiFileStatsExtractor.java | 95 +++++++++++++++-------
2 files changed, 76 insertions(+), 31 deletions(-)
diff --git
a/xtable-core/src/main/java/org/apache/xtable/hudi/HudiFileStatsExtractor.java
b/xtable-core/src/main/java/org/apache/xtable/hudi/HudiFileStatsExtractor.java
index 708b336c..f5ed1195 100644
---
a/xtable-core/src/main/java/org/apache/xtable/hudi/HudiFileStatsExtractor.java
+++
b/xtable-core/src/main/java/org/apache/xtable/hudi/HudiFileStatsExtractor.java
@@ -177,7 +177,7 @@ public class HudiFileStatsExtractor {
}
if (!filesWithoutStats.isEmpty()) {
log.warn(
- "{} file(s) had no column stats in the metadata table for table {};
falling back to parquet footers",
+ "{} file(s) had missing or unusable column stats in the metadata
table for table {}; falling back to parquet footers",
filesWithoutStats.size(),
metaClient.getBasePath());
withStats.addAll(
@@ -201,8 +201,14 @@ public class HudiFileStatsExtractor {
fileStats.stream()
.map(pair -> getColumnStatFromHudiStat(pair.getLeft(),
pair.getRight()))
.collect(CustomCollectors.toList(fileStats.size()));
- long recordCount = getMaxFromColumnStats(columnStats).orElse(0L);
- return
Optional.of(file.toBuilder().columnStats(columnStats).recordCount(recordCount).build());
+ // If every column's value count is absent or non-positive (observed for
files written
+ // before column-stats indexing was enabled), getMaxFromColumnStats
returns empty and this
+ // is treated the same as "no stats" instead of emitting recordCount=0, so
the caller falls
+ // back to reading the authoritative row count from the Parquet footer.
+ return getMaxFromColumnStats(columnStats)
+ .map(
+ recordCount ->
+
file.toBuilder().columnStats(columnStats).recordCount(recordCount).build());
}
private Optional<Long> getMaxFromColumnStats(List<ColumnStat> columnStats) {
diff --git
a/xtable-core/src/test/java/org/apache/xtable/hudi/TestHudiFileStatsExtractor.java
b/xtable-core/src/test/java/org/apache/xtable/hudi/TestHudiFileStatsExtractor.java
index 45835e90..bc6dfc5e 100644
---
a/xtable-core/src/test/java/org/apache/xtable/hudi/TestHudiFileStatsExtractor.java
+++
b/xtable-core/src/test/java/org/apache/xtable/hudi/TestHudiFileStatsExtractor.java
@@ -245,20 +245,7 @@ public class TestHudiFileStatsExtractor {
void columnStatsWithMetadataTableMissingFallsBackToParquetFooters(@TempDir
Path tempDir) {
List<InternalDataFile> inputFiles = generateInputFiles(tempDir, 1);
- 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);
+ HoodieTableMetaClient mockMetaClient = createMockMetaClient(tempDir);
HoodieTableMetadata mockMetadataTable = mock(HoodieTableMetadata.class);
when(mockMetadataTable.getColumnStats(any(),
anyString())).thenReturn(Collections.emptyMap());
@@ -281,20 +268,7 @@ public class TestHudiFileStatsExtractor {
Pair<String, String> fileWithStatsPair =
Pair.of("", new
org.apache.hadoop.fs.Path(fileWithStats.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);
+ HoodieTableMetaClient mockMetaClient = createMockMetaClient(tempDir);
// Metadata table only returns stats for fileWithStats; fileWithoutStats
is missing entirely
HoodieTableMetadata mockMetadataTable = mock(HoodieTableMetadata.class);
@@ -347,6 +321,49 @@ public class TestHudiFileStatsExtractor {
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());
+
+ HoodieTableMetaClient mockMetaClient = createMockMetaClient(tempDir);
+
+ 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());
+
+ validateOutput(output, false);
+ verify(mockMetaClient, times(1)).getStorage();
+ }
+
private List<InternalDataFile> generateInputFiles(Path tempDir, int
numFiles) {
GenericData genericData = GenericData.get();
genericData.addLogicalTypeConversion(new Conversions.DecimalConversion());
@@ -478,6 +495,28 @@ public class TestHudiFileStatsExtractor {
}
}
+ private HoodieTableMetaClient createMockMetaClient(Path tempDir) {
+ HoodieTableConfig mockTableConfig = createMockTableConfig();
+
+ 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);
+ return mockMetaClient;
+ }
+
+ private HoodieTableConfig createMockTableConfig() {
+ HoodieTableConfig mockTableConfig = mock(HoodieTableConfig.class);
+
when(mockTableConfig.isMetadataPartitionAvailable(MetadataPartitionType.COLUMN_STATS))
+ .thenReturn(true);
+ when(mockTableConfig.getTableVersion()).thenReturn(HoodieTableVersion.SIX);
+ return mockTableConfig;
+ }
+
private HoodieRecord<HoodieAvroPayload> buildRecord(GenericRecord record) {
HoodieKey hoodieKey = new HoodieKey(record.get("key").toString(), "");
return new HoodieAvroRecord<>(hoodieKey, new
HoodieAvroPayload(Option.of(record)));
