fix(physical-plan): preserve Exact(0) in FilterExec for null_count, distinct_count and total_byte_size upon empty input - #24000
Conversation
…d distinct_count at num_rows `cap_at_rows` demoted `null_count` and `distinct_count` to inexact unconditionally, so an exactly empty input came out of `FilterExec` with `Exact(0)` for `num_rows`, `total_byte_size` and `byte_size` but `Inexact(0)` for the two counts.
…dictory predicate A contradictory predicate already yielded `Exact(0)` for `num_rows` and per-column `byte_size`, but `total_byte_size` went through the selectivity estimate and came out inexact for any non-empty input.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24000 +/- ##
==========================================
- Coverage 80.84% 80.84% -0.01%
==========================================
Files 1096 1096
Lines 373832 373873 +41
Branches 373832 373873 +41
==========================================
+ Hits 302240 302268 +28
- Misses 53563 53570 +7
- Partials 18029 18035 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
cc: @kosiew and @tschwarzinger, in case you have bandwidth, as you have the full context from #23936 and #23670 |
tschwarzinger
left a comment
There was a problem hiding this comment.
Looks like a good change to me. Thanks again!
| Precision::Exact(0) | ||
| ); | ||
|
|
||
| // A contradictory predicate (`a = 1 AND a = 2`) proves the same for an |
There was a problem hiding this comment.
"proves the same" was a bit confusing for me. Maybe we can rephrase that.
There was a problem hiding this comment.
@asolimando
Thanks for the update! I found a correctness issue around preserving exact empty statistics.
|
|
||
| let total_byte_size = | ||
| input_total_byte_size.with_estimated_selectivity(selectivity); | ||
| let total_byte_size = if is_infeasible { |
There was a problem hiding this comment.
Nice catch making total_byte_size exact for the infeasible case. I think there's still one path that misses the same invariant though.
FilterExec can also prove an empty result when the input already has num_rows: Exact(0). For example, with a satisfiable predicate, num_rows stays Exact(0) and cap_at_rows now correctly produces exact zero row bounded column stats. However, this line still returns input_total_byte_size.with_estimated_selectivity(selectivity), so total_byte_size remains Absent or Inexact.
Could we key this off the computed num_rows == Precision::Exact(0) (or another shared empty output predicate) instead of only is_infeasible? That would preserve the exact empty result invariant regardless of how the filter proves the output is empty.
Could you also add a regression covering an input with num_rows: Exact(0) and total_byte_size: Absent or Inexact?
Which issue does this PR close?
Rationale for this change
Follow-up to #23936 (suggested here).
#23936 made
FilterExecreportnum_rows: Exact(0)for a provably empty result. Two places in the same operator still turn the same proof into an estimate:cap_at_rowsdemotesnull_countanddistinct_countunconditionallytotal_byte_sizeis computed after the "infeasibility branch", so a contradictory predicate (a = 1 AND a = 2) givesExact(0)fornum_rowsand per-columnbyte_sizebut an inexacttotal_byte_sizeWhat changes are included in this PR?
cap_at_rowsnow maps anExact(0)row bound toExact(0)statistics_helperreportstotal_byte_size: Exact(0)for a contradictory predicate instead of routing it through the selectivity estimateAre these changes tested?
Yes, both by extending
test_filter_statistics_preserves_exactly_empty_inputwith two assertions.Both assertions were confirmed to fail without the respective fix. No sqllogictest baselines change.
Are there any user-facing changes?
No breaking changes and no signature changes.
FilterExecreports exact rather than inexact zeros for these statistics, visible inEXPLAINoutput that shows statistics.Disclaimer: I used AI to assist in the code generation, I have manually reviewed the output and it matches my intention and understanding.