Subodh-17 opened a new pull request, #26993: URL: https://github.com/apache/datafusion/pull/26993
> [!IMPORTANT] > > **Fork CI setup:** For pull requests from a fork to `main`, enable workflows in your fork's **Actions** tab before you push. > See the [fork CI setup guide](https://datafusion.apache.org/contributor-guide/#pull-requests-from-forks). ## Which issue does this PR close? Closes #26748 ## Rationale for this change When merging partition statistics via `Statistics::try_merge_iter_with_ndv_fallback` (for example, across multiple parquet files or parallel branches of a `UnionExec`), an all-null partition (`num_rows == null_count`) or empty partition (`num_rows == 0`) has no non-null values that can define a minimum or maximum (`Precision::Absent`). Under existing logic, merging `Precision::Absent` from an all-null partition with known exact bounds (`Precision::Exact(min)`, `Precision::Exact(max)`) causes `precision_min` and `precision_max` to poison the merged result to `Precision::Absent`. In relational semantics, `MIN` and `MAX` aggregates ignore nulls. Erasing valid bounds when an empty or all-null partition is encountered causes downstream query optimizers to lose column range pruning opportunities. ## What changes are included in this PR? 1. **Helper Function `is_known_all_null`:** Detects when a partition is provably empty (`num_rows == Exact(0)`) or provably all-null (`num_rows == Exact(N)` and `null_count == Exact(N)`). Inexact estimates or absent row/null counts are not treated as all-null. 2. **Order-Independent Bound Initialization:** - In `Statistics::try_merge_iter_with_ndv_fallback`, tracks `has_non_null_rows: Vec<bool>` across columns. - If the first partition is all-null or empty, resets its min/max to `Precision::Absent` and marks `has_non_null_rows` false for that column. - The first partition with non-null rows initializes `min_value` and `max_value`. - Subsequent all-null/empty partitions bypass the min/max update, preserving established bounds regardless of partition ordering. 3. **Sound Conservatism:** If `null_count` is unknown (`Precision::Absent`) while `num_rows > 0`, non-null values cannot be disproven, so standard conservative degradation to `Precision::Absent` is preserved. ## What is the testing strategy for this PR? Added comprehensive unit test `test_try_merge_iter_ignores_all_null_and_empty_inputs_for_min_max` in `datafusion/common/src/stats.rs` covering: - Non-null partition followed by all-null partition - All-null partition followed by non-null partition - Empty partition combined with non-null partition (both orderings) - Multi-partition sequence with interleaved all-null partitions - Unknown null count with absent min/max (verified conservative poisoning to `Absent`) - All-null-only inputs (verified bounds remain `Absent`) - Empty-only inputs (verified bounds remain `Absent`) - Exact-zero value preservation (integers and floating point `0.0`/`-0.0`) Validation results: - `cargo fmt --check -p datafusion-common`: PASS (clean formatting) - `cargo test -p datafusion-common --lib stats`: PASS (78 passed; 0 failed) - `cargo clippy -p datafusion-common -- -D warnings`: PASS (0 warnings) - `cargo test -p datafusion-physical-plan --lib union`: PASS (28 passed; 0 failed) ## Are there any user-facing changes? No breaking changes to public APIs. Fixes a statistics degradation bug where table min/max statistics were unnecessarily collapsed to `Precision::Absent`. -- 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]
