Repository navigation
fix: use null counts to estimate IS NULL and IS NOT NULL - #25648
kumarUjjawal wants to merge 1 commit into
Conversation
|
run benchmark tpcds tpch sql_planner compute_statistics |
|
run benchmark imdb |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark sql_plannerResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark imdbResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark compute_statisticsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark tpchCPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark tpcdsCPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark compute_statisticsCPU Details (lscpu)Details
Resource Usagecompute_statistics — base (merge-base)
compute_statistics — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark imdbCPU Details (lscpu)Details
Resource Usageimdb — base (merge-base)
imdb — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing kumar/null-check-statistics (227581d) to 714956b (merge-base) diff Run configurationrun benchmark sql_plannerCPU Details (lscpu)Details
Resource Usagesql_planner — base (merge-base)
sql_planner — branch
File an issue against this benchmark runner |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #25648 +/- ##
==========================================
+ Coverage 82.58% 82.60% +0.02%
==========================================
Files 1142 1142
Lines 440884 441864 +980
Branches 440884 441864 +980
==========================================
+ Hits 364088 365019 +931
+ Misses 54799 54796 -3
- Partials 21997 22049 +52 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| Some(PartialJoinStatistics { | ||
| num_rows: *cardinality.get_value()?, | ||
| num_rows: inner_rows |
There was a problem hiding this comment.
Thanks for working on this! Could we split this into smaller PRs? The original issue is about using null counts for bare IS NULL / IS NOT NULL filters, but this also changes outer-join cardinality estimation, fetch-limit accounting, and singleton-statistics precision.
Some join null-count fixes may be prerequisites; those could go first, followed by the focused null-filter change. The independent fetch and precision changes could be separate PRs. That would make the assumptions and tests much easier to review.
There was a problem hiding this comment.
Split into three independent PRs against main:
- fix: account for row duplication and padding in join null counts #25861: join cardinality and null counts
- fix: apply pushed-down fetch to filter statistics #25862: filter fetch accounting
- fix: preserve uncertainty in singleton filter statistics #25863: singleton precision
The null-filter change is now a separate final commit: 78bdfbc. I am keeping #25648 as a draft until the supporting PRs merge, then I will rebase away their commits. Its Files Changed tab still includes them for now because GitHub does not support native stacks across forks.
Each independent branch passed the physical-plan tests and clippy. The combined branch also passed both affected SQL logic test files, and the singleton PR passed its Parquet file independently.
227581d to
78bdfbc
Compare
## Which issue does this PR close? Related to apache#25620. Split from apache#25648 following [the review request](apache#25648 (comment)). This PR can be reviewed and merged independently. ## Rationale for this change An equality filter can narrow a column's bounds to one value without proving that any row contains that value. With input bounds [1, 100] and actual values 1 and 100, `a = 5` returns no rows, but currently reports an exact distinct count of one when its estimated row count is positive. ## What changes are included in this PR? Report `Inexact(1)` for a singleton domain when the filtered row count is an estimate or is unknown. Preserve exact zero for an exactly empty output and exact one when an exact positive row count proves that the singleton is present. Add an execution regression for the absent value and update affected unit-test and Parquet EXPLAIN expectations. ## What is the testing strategy for this PR? Passed on this independent branch: - `cargo test --profile ci -p datafusion-physical-plan --lib` (2,364 tests) - `cargo clippy --profile ci -p datafusion-physical-plan --all-targets --all-features -- -D warnings` - `cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests -- parquet_statistics.slt` - `cargo fmt --all` and `git diff --check` Benchmarks were not rerun for this split. This extracts the singleton-precision implementation already present in apache#25648. ## Are there any user-facing changes? EXPLAIN reports an estimated distinct count when a matching row is not proven. No public API changes.
## Which issue does this PR close? Related to apache#25620. Split from apache#25648 following [the review request](apache#25648 (comment)). This PR can be reviewed and merged independently. ## Rationale for this change A pushed-down `fetch` stops each FilterExec partition after a bounded number of rows, but filter statistics currently ignore it. A filter with 100 matching rows and `fetch=3` still estimates 100 rows in a single partition. This can distort downstream planning decisions. ## What changes are included in this PR? Apply the per-partition fetch in both the built-in filter statistics and FilterStatisticsProvider. Adjust row and byte estimates while keeping empty, all-null, and proven singleton column statistics consistent. Overall statistics remain estimates when rows may be distributed unevenly across partitions. Add independent execution tests with one and two partitions, plus tests for zero fetch, all-null columns, singleton preservation, and the provider path. ## What is the testing strategy for this PR? Passed on this independent branch: - `cargo test --profile ci -p datafusion-physical-plan --lib` (2,367 tests) - `cargo clippy --profile ci -p datafusion-physical-plan --all-targets --all-features -- -D warnings` - `cargo fmt --all` and `git diff --check` Benchmarks were not rerun for the split. This extracts the fetch implementation already present in apache#25648; its earlier combined measurements are not isolated measurements of this PR. ## Are there any user-facing changes? EXPLAIN statistics account for a fetch pushed into FilterExec. Plan choices can change as a result. No public API changes.
## Which issue does this PR close? Related to apache#25620. Split from apache#25648 following [the review request](apache#25648 (comment)). This PR can be reviewed and merged independently. ## Rationale for this change An equality filter can narrow a column's bounds to one value without proving that any row contains that value. With input bounds [1, 100] and actual values 1 and 100, `a = 5` returns no rows, but currently reports an exact distinct count of one when its estimated row count is positive. ## What changes are included in this PR? Report `Inexact(1)` for a singleton domain when the filtered row count is an estimate or is unknown. Preserve exact zero for an exactly empty output and exact one when an exact positive row count proves that the singleton is present. Add an execution regression for the absent value and update affected unit-test and Parquet EXPLAIN expectations. ## What is the testing strategy for this PR? Passed on this independent branch: - `cargo test --profile ci -p datafusion-physical-plan --lib` (2,364 tests) - `cargo clippy --profile ci -p datafusion-physical-plan --all-targets --all-features -- -D warnings` - `cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests -- parquet_statistics.slt` - `cargo fmt --all` and `git diff --check` Benchmarks were not rerun for this split. This extracts the singleton-precision implementation already present in apache#25648. ## Are there any user-facing changes? EXPLAIN reports an estimated distinct count when a matching row is not proven. No public API changes.
## Which issue does this PR close? Related to apache#25620. Split from apache#25648 following [the review request](apache#25648 (comment)). This PR can be reviewed and merged independently. ## Rationale for this change A pushed-down `fetch` stops each FilterExec partition after a bounded number of rows, but filter statistics currently ignore it. A filter with 100 matching rows and `fetch=3` still estimates 100 rows in a single partition. This can distort downstream planning decisions. ## What changes are included in this PR? Apply the per-partition fetch in both the built-in filter statistics and FilterStatisticsProvider. Adjust row and byte estimates while keeping empty, all-null, and proven singleton column statistics consistent. Overall statistics remain estimates when rows may be distributed unevenly across partitions. Add independent execution tests with one and two partitions, plus tests for zero fetch, all-null columns, singleton preservation, and the provider path. ## What is the testing strategy for this PR? Passed on this independent branch: - `cargo test --profile ci -p datafusion-physical-plan --lib` (2,367 tests) - `cargo clippy --profile ci -p datafusion-physical-plan --all-targets --all-features -- -D warnings` - `cargo fmt --all` and `git diff --check` Benchmarks were not rerun for the split. This extracts the fetch implementation already present in apache#25648; its earlier combined measurements are not isolated measurements of this PR. ## Are there any user-facing changes? EXPLAIN statistics account for a fetch pushed into FilterExec. Plan choices can change as a result. No public API changes.
## Which issue does this PR close? Related to apache#25620. Split from apache#25648 following [the review request](apache#25648 (comment)). This PR can be reviewed and merged independently. ## Rationale for this change An equality filter can narrow a column's bounds to one value without proving that any row contains that value. With input bounds [1, 100] and actual values 1 and 100, `a = 5` returns no rows, but currently reports an exact distinct count of one when its estimated row count is positive. ## What changes are included in this PR? Report `Inexact(1)` for a singleton domain when the filtered row count is an estimate or is unknown. Preserve exact zero for an exactly empty output and exact one when an exact positive row count proves that the singleton is present. Add an execution regression for the absent value and update affected unit-test and Parquet EXPLAIN expectations. ## What is the testing strategy for this PR? Passed on this independent branch: - `cargo test --profile ci -p datafusion-physical-plan --lib` (2,364 tests) - `cargo clippy --profile ci -p datafusion-physical-plan --all-targets --all-features -- -D warnings` - `cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests -- parquet_statistics.slt` - `cargo fmt --all` and `git diff --check` Benchmarks were not rerun for this split. This extracts the singleton-precision implementation already present in apache#25648. ## Are there any user-facing changes? EXPLAIN reports an estimated distinct count when a matching row is not proven. No public API changes.
## Which issue does this PR close? Related to apache#25620. Split from apache#25648 following [the review request](apache#25648 (comment)). This PR can be reviewed and merged independently. ## Rationale for this change A pushed-down `fetch` stops each FilterExec partition after a bounded number of rows, but filter statistics currently ignore it. A filter with 100 matching rows and `fetch=3` still estimates 100 rows in a single partition. This can distort downstream planning decisions. ## What changes are included in this PR? Apply the per-partition fetch in both the built-in filter statistics and FilterStatisticsProvider. Adjust row and byte estimates while keeping empty, all-null, and proven singleton column statistics consistent. Overall statistics remain estimates when rows may be distributed unevenly across partitions. Add independent execution tests with one and two partitions, plus tests for zero fetch, all-null columns, singleton preservation, and the provider path. ## What is the testing strategy for this PR? Passed on this independent branch: - `cargo test --profile ci -p datafusion-physical-plan --lib` (2,367 tests) - `cargo clippy --profile ci -p datafusion-physical-plan --all-targets --all-features -- -D warnings` - `cargo fmt --all` and `git diff --check` Benchmarks were not rerun for the split. This extracts the fetch implementation already present in apache#25648; its earlier combined measurements are not isolated measurements of this PR. ## Are there any user-facing changes? EXPLAIN statistics account for a fetch pushed into FilterExec. Plan choices can change as a result. No public API changes.
…che#25861) ## Which issue does this PR close? Related to apache#25620. Split from apache#25648 following [the review request](apache#25648 (comment)). This PR can be reviewed and merged independently. ## Rationale for this change Joins repeat or drop input rows, and outer joins add NULL padding. Copying each input column's null count unchanged can therefore produce inconsistent output statistics. For example, a left join can repeat a NULL in a non-key left column many times, while an unmatched left row introduces NULLs in every right column. ## What changes are included in this PR? - Estimate inner and outer join null counts from the rows carrying each side's values and the rows added as NULL padding. - Estimate unmatched input rows using the existing semi-join estimator. Inner-join row counts count matching pairs, so they cannot alone determine how many input rows are unmatched. - Account for NULL keys under ordinary and null-safe equality, while retaining unknown contributions and conservative precision. - Rescale null counts when a statistics provider replaces the row estimate. Cover projected joins and swapped inputs. The semi/anti-join estimator itself is unchanged. The rules follow join semantics and do not contain benchmark-specific cases. ## What is the testing strategy for this PR? Added unit tests cover NULL padding, repeated matches, unknown counts, null-safe keys, projection, input swapping, and provider rescaling. Existing cardinality assertions remain in place. Passed on this independent branch: - `cargo test --profile ci -p datafusion-physical-plan --lib` (2,369 tests) - `cargo clippy --profile ci -p datafusion-physical-plan --all-targets --all-features -- -D warnings` - `cargo fmt --all` and `git diff --check` Benchmarks were not rerun for the split. This extracts the join implementation already present in apache#25648; its earlier combined measurements are not isolated measurements of this PR. ## Are there any user-facing changes? Join row and null-count estimates can change, which can affect EXPLAIN output and plan choices. No public API changes.
78bdfbc to
584bc64
Compare
|
run benchmark tpcds tpch sql_planner compute_statistics |
|
@gabotechs last one to finish this |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing kumar/null-check-statistics (584bc64) to 2717e25 (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing kumar/null-check-statistics (584bc64) to 2717e25 (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing kumar/null-check-statistics (584bc64) to 2717e25 (merge-base) diff Run configurationrun benchmark sql_plannerResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing kumar/null-check-statistics (584bc64) to 2717e25 (merge-base) diff Run configurationrun benchmark compute_statisticsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing kumar/null-check-statistics (584bc64) to 2717e25 (merge-base) diff Run configurationrun benchmark tpchCPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing kumar/null-check-statistics (584bc64) to 2717e25 (merge-base) diff Run configurationrun benchmark tpcdsCPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing kumar/null-check-statistics (584bc64) to 2717e25 (merge-base) diff Run configurationrun benchmark compute_statisticsCPU Details (lscpu)Details
Resource Usagecompute_statistics — base (merge-base)
compute_statistics — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing kumar/null-check-statistics (584bc64) to 2717e25 (merge-base) diff Run configurationrun benchmark sql_plannerCPU Details (lscpu)Details
Resource Usagesql_planner — base (merge-base)
sql_planner — branch
File an issue against this benchmark runner |
Which issue does this PR close?
Closes #25620.
Rationale for this change
Bare
IS NULLandIS NOT NULLfilters use the default selectivity even when the input has a null count. For five rows containing two NULLs, both filters estimate one row instead of two and three.What changes are included in this PR?
Use available null counts to estimate bare column null checks, while retaining the existing fallback for missing statistics and other predicates. Keep the output column statistics consistent with all-null, non-null, and empty results. Preserve the checked column's distinct count through the statistics provider when all its non-null values survive.
Add regression coverage for exact and estimated counts, missing statistics, projection, fetch, and null filters above outer joins.
The supporting changes in #25861, #25862, and #25863 have merged. This PR now contains only the null-filter change on current
main.What is the testing strategy for this PR?
The null-count and statistics-provider regressions fail against unchanged
mainproduction code.Compared 21 optimized
compute_statisticscoalesce, filter, and mixed-chain cases againstmainat2717e25625ff0c51c1805f8df52d317979db74ec, using therelease-nonltoprofile and 30 samples per case. Median time changes were +0.4% for coalesce controls, -1.1% for filter chains, and -5.1% for mixed chains. These local measurements cover statistics computation.Are there any user-facing changes?
Null-check row estimates and related EXPLAIN statistics improve. Plan choices can change as a result. No public API changes.