Skip to content

docs: clarify what PushedDown::Yes and PushedDown::No mean - #25670

Merged
alamb merged 1 commit into
apache:mainfrom
pydantic:pushed-down-no-docs
Sep 24, 2026
Merged

alamb merged 1 commit into
apache:mainfrom
pydantic:pushed-down-no-docs

Conversation

@adriangb

@adriangb adriangb commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

The docs say PushedDown::No means "the predicate could not be pushed down into the child node". This reads as "the child ignores the filter". But a child that replies No can still use the filter, for example for statistics pruning.

What changes are included in this PR?

Doc comments on PushedDown:

Reply Must the parent evaluate the filter? Can the child use the filter?
Yes No. The child applies it exactly. Yes, exactly
No Yes Yes, in an inexact way (for example statistics pruning), or not at all

What is the testing strategy for this PR?

Documentation only. cargo doc -p datafusion-physical-plan builds with -D warnings.

Are there any user-facing changes?

No.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Sep 24, 2026
@codecov-commenter

codecov-commenter commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.51%. Comparing base (95bb0a0) to head (b0a974a).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25670      +/-   ##
==========================================
+ Coverage   82.48%   82.51%   +0.02%     
==========================================
  Files        1140     1140              
  Lines      437614   438443     +829     
  Branches   437614   438443     +829     
==========================================
+ Hits       360986   361763     +777     
- Misses      54834    54836       +2     
- Partials    21794    21844      +50     

☔ 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.

@jayzhan211 jayzhan211 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.

👍🏻

@adriangb adriangb changed the title docs: clarify what PushedDown::No means for filter pushdown docs: clarify what PushedDown::Yes and PushedDown::No mean Sep 24, 2026
@adriangb

Copy link
Copy Markdown
Contributor Author

FYI @jayzhan211 I toned it down a bit about implementation details and that Inexact is not needed. It's not a smaller change. I will leave this open for a day for comment regarldess. Thanks for reviewing.

`PushedDown::Yes` means the child applies the predicate exactly, so the
parent does not need to evaluate it again. `PushedDown::No` means the parent
must still evaluate it. A child that replies `No` can still use the filter
in an inexact way, for example for statistics pruning. Document this.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alamb alamb added the documentation Improvements or additions to documentation label Sep 24, 2026

@alamb alamb 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.

📖 ✅

@alamb
alamb added this pull request to the merge queue Sep 24, 2026
Merged via the queue into apache:main with commit 1be6b04 Sep 24, 2026
42 checks passed
diegoQuinas pushed a commit to diegoQuinas/datafusion that referenced this pull request Sep 24, 2026
…e#25670)

## Which issue does this PR close?

- Related to apache#18856 and apache#24528. This PR does not change the API. It only
documents the two states that exist.

## Rationale for this change

The docs say `PushedDown::No` means "the predicate could not be pushed
down into the child node". This reads as "the child ignores the filter".
But a child that replies `No` can still use the filter, for example for
statistics pruning.

## What changes are included in this PR?

Doc comments on `PushedDown`:

| Reply | Must the parent evaluate the filter? | Can the child use the
filter? |
|---|---|---|
| `Yes` | No. The child applies it exactly. | Yes, exactly |
| `No` | Yes | Yes, in an inexact way (for example statistics pruning),
or not at all |

## What is the testing strategy for this PR?

Documentation only. `cargo doc -p datafusion-physical-plan` builds with
`-D warnings`.

## Are there any user-facing changes?

No.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants