[Data] - Predicate Pushdown - Push predicate exprs past eligible operators#58555

Merged

bveeramani merged 11 commits intoray-project:masterfrom

goutamvenkat-anyscale:goutam/predicate_pushdown_passthrough

Nov 14, 2025

Contributor

goutamvenkat-anyscale commented Nov 12, 2025 •

edited

Loading

Description

Push Filter past Join (depends on the join op), Filter into Union branches, Filter past projections (accounting for renames), past all shuffle ops.

Related issues

Link related issues: "Fixes #1234", "Closes #1234", or "Related to #1234".

Additional information

Optional: Add implementation details, API changes, usage examples, screenshots, etc.


          [Data] - Predicate Pushdown - Push predicate exprs past eligible oper…

9656ad3

…ators

Signed-off-by: Goutam <[email protected]>

goutamvenkat-anyscale requested a review from a team as a code owner

November 12, 2025 04:21

gemini-code-assist bot reviewed

View reviewed changes

Contributor

gemini-code-assist bot left a comment

Code Review

This pull request introduces a significant and well-designed refactoring of the predicate pushdown logic. By moving to a trait-based system with PredicatePushable and PredicatePushdownBehavior, the logic becomes much more extensible and maintainable, which is a great improvement. The changes are consistently applied across various operators, and the new tests are comprehensive and well-structured.

I've found one important issue in the new _clone_op_with_new_inputs helper that could lead to an inconsistent operator graph. I've also left a comment on a potentially confusing implementation in the new PredicatePushable interface.

Overall, this is excellent work that improves the foundation of the query optimizer.

python/ray/data/_internal/logical/rules/predicate_pushdown.py Show resolved Hide resolved

python/ray/data/_internal/logical/interfaces/logical_operator.py Outdated Show resolved Hide resolved

ray-gardener bot added the data label


          Some cleanup

0d8b019

Signed-off-by: Goutam <[email protected]>

goutamvenkat-anyscale added the go label


          rename class

f55eba3

Signed-off-by: Goutam <[email protected]>

cursor bot reviewed

View reviewed changes

python/ray/data/_internal/logical/operators/join_operator.py Outdated Show resolved Hide resolved

goutamvenkat-anyscale commented

View reviewed changes

python/ray/data/_internal/logical/interfaces/logical_operator.py Outdated Show resolved Hide resolved


          Merge branch 'master' into goutam/predicate_pushdown_passthrough

3a1cf68

bveeramani reviewed

View reviewed changes

python/ray/data/_internal/logical/rules/predicate_pushdown.py Outdated

                           return op
                       input_op = op.input_dependencies[0]
+                      predicate_expr = op._predicate_expr

Member

bveeramani Nov 12, 2025

The annotation for op is LogicalOperator, but I don't think LogicalOperator is guaranteed to have a _predicate_expr attribute. Is there a risk of attribute errors?

Contributor Author

goutamvenkat-anyscale Nov 12, 2025

Oh we check if it's an expression based Filter in the if statement above. I can make the typing here explicit to make it easier.

python/ray/data/_internal/logical/rules/predicate_pushdown.py Outdated

                           return input_op.apply_predicate(predicate_expr)
+                      # Case 2: Check if operator allows predicates to pass through
+                      if isinstance(input_op, PredicatePassThrough):

Member

bveeramani Nov 12, 2025

Nit: LogicalOperatorSupportsPredicatePushdown and PredicatePassThrough appear to follow very different naming convention. Any reason not to use similar naming patterns (e.g., PredicatePushdown/PredicatePassThrough or LogicalOperatorSupportsPredicatePushdown/LogicalOperatorSupportsPredicatePassThrough)?

Contributor Author

goutamvenkat-anyscale Nov 12, 2025

I don't have a strong opinion on this one. I can change this to LogicalOperatorSupportsPredicatePassThrough to be consistent.

python/ray/data/_internal/logical/rules/predicate_pushdown.py Outdated

+                          if behavior == PredicatePushdownBehavior.PASSTHROUGH:
+                              # Push filter through and recursively try to push further
+                              new_filter = Filter(
+                                  input_op.input_dependencies[0],

Member

bveeramani Nov 12, 2025

(Here and below) Are we always guaranteed to have one input dependency in this case? If this is an assumption we're making, add assertion?

Contributor Author

goutamvenkat-anyscale Nov 12, 2025

Yes added an assertion

python/ray/data/_internal/logical/rules/predicate_pushdown.py

-                          branch₁ ─> Filter(predicate) ─┐
-                          branch₂ ─> Filter(predicate) ─┤ Union
-                          branch₃ ─> Filter(predicate) ─┘
+                  def _push_filter_through_conditionally(

Member

bveeramani Nov 12, 2025

This _push_filter_through_conditionally/PredicatePushdownBehavior .CONDITIONAL abstractions confused me because they sounds like a generic abstractions, but the implementation is specific to joins.

Don't really have a good alternative suggestion, but thought it was worth calling out.

Contributor Author

goutamvenkat-anyscale Nov 12, 2025 •

edited

Loading

For now it's specific to joins. But this could apply to Intersect (not yet in RD), Window operator etc.

python/ray/data/tests/test_predicate_pushdown.py

		)


		class TestPredicatePushdownIntoRead:

Member

bveeramani Nov 12, 2025

Are there tests for the join case anywhere?

Contributor Author

goutamvenkat-anyscale Nov 12, 2025

Yes in test_join

Member

bveeramani Nov 14, 2025

Ah, missed it

python/ray/data/tests/test_predicate_pushdown.py Outdated

Comment on lines +376 to +378

+                      # Verify plan: all filters pushed into Read, passthrough ops remain
+                      plan = _get_optimized_plan(ds)
+                      assert "Filter[Filter" not in plan, f"No Filter operators should remain: {plan}"

Member

bveeramani Nov 12, 2025

Here and elsewhere -- would prefer to not test against the string representations because that's historically been pretty brittle.

If it'd make sense to do so, one option is to add a utility function to LogicalPlan to see it structurally matches another.

Contributor Author

goutamvenkat-anyscale Nov 13, 2025

I added some helper functions in test utils to replace the string assertions with class assertions

goutamvenkat-anyscale added 2 commits

November 12, 2025 14:11


          Fix join bug

ca1e960

Signed-off-by: Goutam <[email protected]>


          Some renaming

2c9586f

Signed-off-by: Goutam <[email protected]>

cursor bot reviewed

View reviewed changes

python/ray/data/_internal/logical/operators/join_operator.py Outdated Show resolved Hide resolved

goutamvenkat-anyscale added 2 commits

November 12, 2025 14:22


          Remove dead code

2f6c7de

Signed-off-by: Goutam <[email protected]>


          Address comments

435ae19

Signed-off-by: Goutam <[email protected]>

cursor bot reviewed

View reviewed changes

python/ray/data/_internal/logical/operators/join_operator.py Outdated Show resolved Hide resolved

goutamvenkat-anyscale added 3 commits

November 12, 2025 18:53


          Bug fix

b911da5

Signed-off-by: Goutam <[email protected]>


          Clean up

1023b5b

Signed-off-by: Goutam <[email protected]>


          cleanup

0ea3c88

Signed-off-by: Goutam <[email protected]>

bveeramani approved these changes

View reviewed changes

python/ray/data/tests/test_predicate_pushdown.py

		)


		class TestPredicatePushdownIntoRead:

Member

bveeramani Nov 14, 2025

Ah, missed it

bveeramani merged commit 7a05d43 into ray-project:master

6 checks passed

goutamvenkat-anyscale deleted the goutam/predicate_pushdown_passthrough branch

November 14, 2025 19:43

richardliaw mentioned this pull request

Ray Data Q4 Roadmap + Wishlist #58665

Open

ArturNiederfahrenhorst pushed a commit to ArturNiederfahrenhorst/ray that referenced this pull request


          [Data] - Predicate Pushdown - Push predicate exprs past eligible oper…

fede122

…ators (ray-project#58555)

## Description
Push `Filter` past Join (depends on the join op), `Filter` into Union
branches, `Filter` past projections (accounting for renames), past all
shuffle ops.
## Related issues
> Link related issues: "Fixes ray-project#1234", "Closes ray-project#1234", or "Related to
ray-project#1234".

## Additional information
> Optional: Add implementation details, API changes, usage examples,
screenshots, etc.

---------

Signed-off-by: Goutam <[email protected]>

Aydin-ab pushed a commit to Aydin-ab/ray-aydin that referenced this pull request


          [Data] - Predicate Pushdown - Push predicate exprs past eligible oper…

f2d75da

…ators (ray-project#58555)

## Description
Push `Filter` past Join (depends on the join op), `Filter` into Union
branches, `Filter` past projections (accounting for renames), past all
shuffle ops.
## Related issues
> Link related issues: "Fixes ray-project#1234", "Closes ray-project#1234", or "Related to
ray-project#1234".

## Additional information
> Optional: Add implementation details, API changes, usage examples,
screenshots, etc.

---------

Signed-off-by: Goutam <[email protected]>
Signed-off-by: Aydin Abiar <[email protected]>

ykdojo pushed a commit to ykdojo/ray that referenced this pull request


          [Data] - Predicate Pushdown - Push predicate exprs past eligible oper…

474a278

…ators (ray-project#58555)

## Description
Push `Filter` past Join (depends on the join op), `Filter` into Union
branches, `Filter` past projections (accounting for renames), past all
shuffle ops.
## Related issues
> Link related issues: "Fixes ray-project#1234", "Closes ray-project#1234", or "Related to
ray-project#1234".

## Additional information
> Optional: Add implementation details, API changes, usage examples,
screenshots, etc.

---------

Signed-off-by: Goutam <[email protected]>
Signed-off-by: YK <[email protected]>

SheldonTsen pushed a commit to SheldonTsen/ray that referenced this pull request


          [Data] - Predicate Pushdown - Push predicate exprs past eligible oper…

780c4ed

…ators (ray-project#58555)

## Description
Push `Filter` past Join (depends on the join op), `Filter` into Union
branches, `Filter` past projections (accounting for renames), past all
shuffle ops.
## Related issues
> Link related issues: "Fixes ray-project#1234", "Closes ray-project#1234", or "Related to
ray-project#1234".

## Additional information
> Optional: Add implementation details, API changes, usage examples,
screenshots, etc.

---------

Signed-off-by: Goutam <[email protected]>

Future-Outlier pushed a commit to Future-Outlier/ray that referenced this pull request


          [Data] - Predicate Pushdown - Push predicate exprs past eligible oper…

19ed749

…ators (ray-project#58555)

## Description
Push `Filter` past Join (depends on the join op), `Filter` into Union
branches, `Filter` past projections (accounting for renames), past all
shuffle ops.
## Related issues
> Link related issues: "Fixes ray-project#1234", "Closes ray-project#1234", or "Related to
ray-project#1234".

## Additional information
> Optional: Add implementation details, API changes, usage examples,
screenshots, etc.

---------

Signed-off-by: Goutam <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>

peterxcli pushed a commit to peterxcli/ray that referenced this pull request


          [Data] - Predicate Pushdown - Push predicate exprs past eligible oper…

a519dee

…ators (ray-project#58555)

## Description
Push `Filter` past Join (depends on the join op), `Filter` into Union
branches, `Filter` past projections (accounting for renames), past all
shuffle ops.
## Related issues
> Link related issues: "Fixes ray-project#1234", "Closes ray-project#1234", or "Related to
ray-project#1234".

## Additional information
> Optional: Add implementation details, API changes, usage examples,
screenshots, etc.

---------

Signed-off-by: Goutam <[email protected]>
Signed-off-by: peterxcli <[email protected]>

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

Labels

data go