Skip to content

fix(physical-plan): preserve Exact(0) in FilterExec for null_count, distinct_count and total_byte_size upon empty input - #24000

Open
asolimando wants to merge 2 commits into
apache:mainfrom
asolimando:asolimando/filter-exact-empty-counts
Open

fix(physical-plan): preserve Exact(0) in FilterExec for null_count, distinct_count and total_byte_size upon empty input#24000
asolimando wants to merge 2 commits into
apache:mainfrom
asolimando:asolimando/filter-exact-empty-counts

Conversation

@asolimando

Copy link
Copy Markdown
Member

Which issue does this PR close?

Rationale for this change

Follow-up to #23936 (suggested here).

#23936 made FilterExec report num_rows: Exact(0) for a provably empty result. Two places in the same operator still turn the same proof into an estimate:

  • cap_at_rows demotes null_count and distinct_count unconditionally
  • total_byte_size is computed after the "infeasibility branch", so a contradictory predicate (a = 1 AND a = 2) gives Exact(0) for num_rows and per-column byte_size but an inexact total_byte_size

What changes are included in this PR?

  • cap_at_rows now maps an Exact(0) row bound to Exact(0)
  • statistics_helper reports total_byte_size: Exact(0) for a contradictory predicate instead of routing it through the selectivity estimate

Are these changes tested?

Yes, both by extending test_filter_statistics_preserves_exactly_empty_input with 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. FilterExec reports exact rather than inexact zeros for these statistics, visible in EXPLAIN output 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.

…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.
@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Jul 30, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.84%. Comparing base (455a3ad) to head (7a0b6bb).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/physical-plan/src/filter.rs 96.00% 0 Missing and 2 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@asolimando

Copy link
Copy Markdown
Member Author

cc: @kosiew and @tschwarzinger, in case you have bandwidth, as you have the full context from #23936 and #23670

@tschwarzinger tschwarzinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"proves the same" was a bit confusing for me. Maybe we can rephrase that.

@kosiew kosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants