danny0405 commented on code in PR #20295:
URL: https://github.com/apache/hudi/pull/20295#discussion_r4256834435
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/source/stats/TestColumnStatsIndex.java:
##########
@@ -164,6 +172,60 @@ void testReadColumnStatsForCandidatePartitions(String
partition) throws Exceptio
}
}
+ @Test
+ void testCandidateFilesWithMissingColumnStatsAfterIndexExpansion() throws
Exception {
+ final String path = tempFile.getAbsolutePath();
+ Configuration conf = TestConfigurations.getDefaultConf(path);
+ conf.set(FlinkOptions.TABLE_TYPE, "COPY_ON_WRITE");
+ conf.set(FlinkOptions.METADATA_ENABLED, true);
+ conf.set(FlinkOptions.READ_DATA_SKIPPING_ENABLED, true);
+
conf.setString(HoodieMetadataConfig.ENABLE_METADATA_INDEX_COLUMN_STATS.key(),
"true");
+
conf.setString(HoodieMetadataConfig.ENABLE_METADATA_INDEX_PARTITION_STATS.key(),
"false");
Review Comment:
Could we also cover incomplete statistics through
`PartitionStatsIndex.computeCandidatePartitions()`? Both new index-expansion
tests disable partition statistics, but `PartitionStatsIndex` inherits the
changed transposition and probe logic. A case with a matching partition that
has age statistics but no name statistics would verify that partition pruning
retains it before file pruning runs.
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/source/prune/ColumnStatsProbe.java:
##########
@@ -60,6 +60,14 @@ public class ColumnStatsProbe implements Serializable {
* @return true if the index row should be considered as a candidate
*/
public boolean test(RowData indexRow, RowType.RowField[] queryFields) {
+ // Retain candidates with missing column statistics rather than treating
unknown values as nulls.
+ for (int i = 0; i < queryFields.length; i++) {
+ // The row starts with file name and value count, followed by (min, max,
null count) per column.
+ // Skip the two leading fields and i triples, then use offset 2 to check
this column's null count.
+ if (indexRow.isNullAt(2 + i * 3 + 2)) {
+ return true;
Review Comment:
Nonblocking performance suggestion: this early return also bypasses
predicates whose statistics are complete. For example, a file with age in [20,
30] and missing name statistics is retained for `age > 100 AND name = 'alice'`,
even though age alone excludes it. After index expansion this can force scans
of many historical files, and the shared partition-statistics path is affected
too. The PR description acknowledges this tradeoff, so keeping the conservative
guard for this correctness fix seems reasonable. Could we add a regression case
documenting this behavior and consider evaluating independent top-level
conjuncts with complete statistics in a follow-up? If pursuing that
optimization, avoid simply returning true from missing-statistics leaf
evaluators: the existing `Not.eval()` negates its child result.
--
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]
