uddhavdave opened a new pull request, #25149: URL: https://github.com/apache/datafusion/pull/25149
## Which issue does this PR close? - Closes #25147. ## Rationale for this change `SELECT MIN(col), MAX(col)` over a schema-evolved Parquet dataset can return a wrong `MIN` when `datafusion.optimizer.enable_aggregate_dynamic_filter_pushdown` is enabled and the query runs with multiple partitions. When one of the files does not contain the aggregated column, the `Partial` aggregate for that partition evaluates to a *typed* null such as `Int64(NULL)` rather than `ScalarValue::Null`. The merge of per-partition bounds into the shared dynamic filter bound only short-circuited on `ScalarValue::Null`. The typed null therefore reached `partial_cmp`, where `None` orders before `Some(_)`, and either replaced a valid shared minimum or blocked a later valid minimum from being recorded. The dynamic filter then lost its lower bound and became just `col > <max>`, which pruned the file holding the true minimum. The result depended on partition scheduling; `target_partitions = 1` or disabling the pushdown returned the correct answer. ## What changes are included in this PR? - `scalar_cmp_null_short_circuit` in `datafusion/physical-plan/src/aggregates/aggregate_stream.rs` now uses `ScalarValue::is_null()` so both untyped and typed nulls are treated as "no bound yet" when merging `MIN`/`MAX` bounds across partitions. ## What is the testing strategy for this PR? - New unit test `scalar_min_max_ignore_typed_nulls` in `aggregate_stream.rs` covers `scalar_min`/`scalar_max` with typed nulls, untyped nulls, and regular values on either side. This fails without the fix and is deterministic. - New sqllogictest case in `datafusion/sqllogictest/test_files/push_down_filter_regression.slt` that builds a three-file Parquet fixture (one file without the aggregated column) and runs `MIN`/`MAX` with `target_partitions = 8`. The file holding the minimum is named to sort last so it is opened after the other partitions publish their bounds. With the fix reverted locally this reproduced the wrong result in 4 of 5 runs; with the fix it passes consistently. Because the failure depends on scheduling, the unit test is the primary regression guard and the slt case documents the end-to-end scenario. `cargo fmt --all`, `cargo clippy --all-targets --all-features -- -D warnings`, and the `push_down_filter_regression` sqllogictest all pass locally. ## Are there any user-facing changes? No API changes. Queries affected by the bug now return the correct `MIN`/`MAX`. -- 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]
