jensholdgaard commented on code in PR #24770:
URL: https://github.com/apache/datafusion/pull/24770#discussion_r3940895289
##########
datafusion/datasource-parquet/src/opener/mod.rs:
##########
@@ -794,10 +800,17 @@ impl ParquetMorselizer {
// Note that if there are statistics for partition columns there will
be overlap,
// but since we use a HashMap, we'll just overwrite the partition
values with the
// constant values from statistics (which should be the same).
- literal_columns.extend(constant_columns_from_stats(
+ let stats_constants = constant_columns_from_stats(
partitioned_file.statistics.as_deref(),
&logical_file_schema,
- ));
+ );
+ let stats_constants_in_predicate = !stats_constants.is_empty()
Review Comment:
> Written by Claude, an AI assistant, on behalf of the PR author.
Agreed, and addressed in 6f86ec7. The flag recorded only that the predicate
*referenced* a column statistics had proven constant, then treated any later
collapse to `false`/NULL as proof that statistics caused it. Those are two
different claims, and the `a = 1 AND missing_b = 2` example separates them.
The check now asks the narrower question at substitution time: substitute
only the statistics-derived constants, simplify that, and take a collapse as
proof. Anything the missing-column adapter or partition folding does afterwards
is irrelevant, because the proof already stands on the file's own statistics.
In the mixed case, `missing_b = 2` is left standing as a real column reference,
nothing collapses, and the file takes the existing missing-column path. That
also drops the `Literal` downcast from `prune_row_groups`, which is now just
the two flags; `collect_columns` stays as the cheap short-circuit so the
simplifier only runs when a stats constant is actually referenced. The
simplifier is given the full table schema, since the substitution deliberately
leaves non-constant columns in place and it types each node as it walks.
`test_no_prune_when_missing_column_collapses_mixed_predicate` covers exactly
that shape: a stats-constant present column plus a missing column, asserting
the statistics branch does not claim the prune and the file is scanned (3 rows
returned, 0 row groups pruned). The `evolved_schema*` and
`test_pushdown_with_missing_*` tests are unchanged and pass.
##########
datafusion/datasource-parquet/src/opener/mod.rs:
##########
@@ -3023,6 +3075,100 @@ mod test {
assert_eq!(num_rows, 0);
Review Comment:
> Written by Claude, an AI assistant, on behalf of the PR author.
Added in 6f86ec7 as
`test_prune_exact_constant_column_false_predicate_from_file_statistics`: exact
`min == max == 7` statistics prove `a` constant, and `a = 8` folds to `false`,
so the row group is pruned on that proof. Together with the all-NULL case, both
literal values the branch accepts are now covered. The
`row_groups_pruned_statistics` accessor is hoisted into a shared test helper,
since all three tests assert on that metric rather than the row count, which
cannot tell "pruned" from "scanned, then row-filtered".
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
