[Data] Disable Limit Pushdown rule in new execution plan optimizer by scottjlee · Pull Request #36831 · ray-project/ray

scottjlee · 2023-06-26T20:47:19Z

Why are these changes needed?

The Limit Pushdown optimization rule, originally implemented in #35950, makes the assumption that Map and MapBatches operators do not change the number of input and output rows. We currently do not have any checks to enforce this condition, so as a result, if this row count invariant condition is not met, it is possible that the output will be incorrect if Limit Pushdown is applied.

This row check was expected to be a relatively small change to be implemented in #36295, but this spawned additional discussion around whether we should enforce this in the first place (particularly around filtering with map_batches, current users' typical use cases, etc).

We will delay implementing the row count invariant condition until Ray 2.7, so for Ray 2.6 release, we will disable the Limit Pushdown rule, and re-enable it once the aforementioned row count invariant discussion is resolved.

Related issue number

Closes #36821

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Scott Lee <sjl@anyscale.com>

…ay-project#36831) The Limit Pushdown optimization rule, originally implemented in ray-project#35950, makes the assumption that `Map` and `MapBatches` operators do not change the number of input and output rows. We currently do not have any checks to enforce this condition, so as a result, if this row count invariant condition is not met, it is possible that the output will be incorrect if Limit Pushdown is applied. This row check was expected to be a relatively small change to be implemented in ray-project#36295, but this spawned additional discussion around whether we should enforce this in the first place (particularly around filtering with map_batches, current users' typical use cases, etc). We will delay implementing the row count invariant condition until Ray 2.7, so for Ray 2.6 release, we will disable the Limit Pushdown rule, and re-enable it once the aforementioned row count invariant discussion is resolved. Signed-off-by: Scott Lee <sjl@anyscale.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>

This PR updates the operator fusion rule to fuse `MapBatches` even if they modify the row counts. The intention of this PR is to preserve the historical operator fusion behavior and avoid introducing regressions. For more details, see the timeline below. --- ### Timeline of Changes | Date | Event | Description | | :--- | :--- | :--- | | **June 8, 2023** | **Limit pushdown added** | Added limit pushdown and a property to `MapBatches` incorrectly stating it doesn't modify row counts. (#35950) | | **June 27, 2023** | **Limit pushdown disabled** | Rule disabled because it incorrectly pushed limits past UDFs that modified row counts. (#36831) | | **April 28, 2025** | **Fusion restricted** | Added logic to stop fusing operators that modify row counts when the downstream has a batch size. `MapBatches` stayed fused only because of its incorrect property (#52570). | | **July 8, 2025** | **Limit pushdown re-enabled with special case** | Re-enabled with a special case to prevent pushing limits past `MapBatches`. ([#39486](#39486)) | | **Oct 24, 2025** | **Special case removed** | Special case removed, re-introducing the bug where limits are pushed past `MapBatches`. ([#57880](#57880)) | | **Feb 2, 2026** | **Property Fix** | Updated `MapBatches` to correctly report it modifies rows by default. This fixed the pushdown bug but broke fusion logic. ([PR #60448](#60448)) | | **Feb 4, 2026** | (This PR) | Add a special-case to preserve the historical `MapBatches` fusion behavior | --- <a href="https://cursor.com/dashboard?tab=bugbot">Cursor Bugbot</a> reviewed your changes and found no issues for commit d99e7b1 --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>

…ct#60756) This PR updates the operator fusion rule to fuse `MapBatches` even if they modify the row counts. The intention of this PR is to preserve the historical operator fusion behavior and avoid introducing regressions. For more details, see the timeline below. --- ### Timeline of Changes | Date | Event | Description | | :--- | :--- | :--- | | **June 8, 2023** | **Limit pushdown added** | Added limit pushdown and a property to `MapBatches` incorrectly stating it doesn't modify row counts. (ray-project#35950) | | **June 27, 2023** | **Limit pushdown disabled** | Rule disabled because it incorrectly pushed limits past UDFs that modified row counts. (ray-project#36831) | | **April 28, 2025** | **Fusion restricted** | Added logic to stop fusing operators that modify row counts when the downstream has a batch size. `MapBatches` stayed fused only because of its incorrect property (ray-project#52570). | | **July 8, 2025** | **Limit pushdown re-enabled with special case** | Re-enabled with a special case to prevent pushing limits past `MapBatches`. ([ray-project#39486](ray-project#39486)) | | **Oct 24, 2025** | **Special case removed** | Special case removed, re-introducing the bug where limits are pushed past `MapBatches`. ([ray-project#57880](ray-project#57880)) | | **Feb 2, 2026** | **Property Fix** | Updated `MapBatches` to correctly report it modifies rows by default. This fixed the pushdown bug but broke fusion logic. ([PR ray-project#60448](ray-project#60448)) | | **Feb 4, 2026** | (This PR) | Add a special-case to preserve the historical `MapBatches` fusion behavior | --- <a href="https://cursor.com/dashboard?tab=bugbot">Cursor Bugbot</a> reviewed your changes and found no issues for commit d99e7b1 --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu> Signed-off-by: tiennguyentony <46289799+tiennguyentony@users.noreply.github.com>

…ct#60756) This PR updates the operator fusion rule to fuse `MapBatches` even if they modify the row counts. The intention of this PR is to preserve the historical operator fusion behavior and avoid introducing regressions. For more details, see the timeline below. --- ### Timeline of Changes | Date | Event | Description | | :--- | :--- | :--- | | **June 8, 2023** | **Limit pushdown added** | Added limit pushdown and a property to `MapBatches` incorrectly stating it doesn't modify row counts. (ray-project#35950) | | **June 27, 2023** | **Limit pushdown disabled** | Rule disabled because it incorrectly pushed limits past UDFs that modified row counts. (ray-project#36831) | | **April 28, 2025** | **Fusion restricted** | Added logic to stop fusing operators that modify row counts when the downstream has a batch size. `MapBatches` stayed fused only because of its incorrect property (ray-project#52570). | | **July 8, 2025** | **Limit pushdown re-enabled with special case** | Re-enabled with a special case to prevent pushing limits past `MapBatches`. ([ray-project#39486](ray-project#39486)) | | **Oct 24, 2025** | **Special case removed** | Special case removed, re-introducing the bug where limits are pushed past `MapBatches`. ([ray-project#57880](ray-project#57880)) | | **Feb 2, 2026** | **Property Fix** | Updated `MapBatches` to correctly report it modifies rows by default. This fixed the pushdown bug but broke fusion logic. ([PR ray-project#60448](ray-project#60448)) | | **Feb 4, 2026** | (This PR) | Add a special-case to preserve the historical `MapBatches` fusion behavior | --- <a href="https://cursor.com/dashboard?tab=bugbot">Cursor Bugbot</a> reviewed your changes and found no issues for commit d99e7b1 --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>

This PR updates the operator fusion rule to fuse `MapBatches` even if they modify the row counts. The intention of this PR is to preserve the historical operator fusion behavior and avoid introducing regressions. For more details, see the timeline below. --- ### Timeline of Changes | Date | Event | Description | | :--- | :--- | :--- | | **June 8, 2023** | **Limit pushdown added** | Added limit pushdown and a property to `MapBatches` incorrectly stating it doesn't modify row counts. (#35950) | | **June 27, 2023** | **Limit pushdown disabled** | Rule disabled because it incorrectly pushed limits past UDFs that modified row counts. (#36831) | | **April 28, 2025** | **Fusion restricted** | Added logic to stop fusing operators that modify row counts when the downstream has a batch size. `MapBatches` stayed fused only because of its incorrect property (#52570). | | **July 8, 2025** | **Limit pushdown re-enabled with special case** | Re-enabled with a special case to prevent pushing limits past `MapBatches`. ([#39486](#39486)) | | **Oct 24, 2025** | **Special case removed** | Special case removed, re-introducing the bug where limits are pushed past `MapBatches`. ([#57880](#57880)) | | **Feb 2, 2026** | **Property Fix** | Updated `MapBatches` to correctly report it modifies rows by default. This fixed the pushdown bug but broke fusion logic. ([PR #60448](#60448)) | | **Feb 4, 2026** | (This PR) | Add a special-case to preserve the historical `MapBatches` fusion behavior | --- <a href="https://cursor.com/dashboard?tab=bugbot">Cursor Bugbot</a> reviewed your changes and found no issues for commit d99e7b1 --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>

This PR updates the operator fusion rule to fuse `MapBatches` even if they modify the row counts. The intention of this PR is to preserve the historical operator fusion behavior and avoid introducing regressions. For more details, see the timeline below. --- ### Timeline of Changes | Date | Event | Description | | :--- | :--- | :--- | | **June 8, 2023** | **Limit pushdown added** | Added limit pushdown and a property to `MapBatches` incorrectly stating it doesn't modify row counts. (#35950) | | **June 27, 2023** | **Limit pushdown disabled** | Rule disabled because it incorrectly pushed limits past UDFs that modified row counts. (#36831) | | **April 28, 2025** | **Fusion restricted** | Added logic to stop fusing operators that modify row counts when the downstream has a batch size. `MapBatches` stayed fused only because of its incorrect property (#52570). | | **July 8, 2025** | **Limit pushdown re-enabled with special case** | Re-enabled with a special case to prevent pushing limits past `MapBatches`. ([#39486](#39486)) | | **Oct 24, 2025** | **Special case removed** | Special case removed, re-introducing the bug where limits are pushed past `MapBatches`. ([#57880](#57880)) | | **Feb 2, 2026** | **Property Fix** | Updated `MapBatches` to correctly report it modifies rows by default. This fixed the pushdown bug but broke fusion logic. ([PR #60448](#60448)) | | **Feb 4, 2026** | (This PR) | Add a special-case to preserve the historical `MapBatches` fusion behavior | --- <a href="https://cursor.com/dashboard?tab=bugbot">Cursor Bugbot</a> reviewed your changes and found no issues for commit d99e7b1 --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>

…ct#60756) This PR updates the operator fusion rule to fuse `MapBatches` even if they modify the row counts. The intention of this PR is to preserve the historical operator fusion behavior and avoid introducing regressions. For more details, see the timeline below. --- ### Timeline of Changes | Date | Event | Description | | :--- | :--- | :--- | | **June 8, 2023** | **Limit pushdown added** | Added limit pushdown and a property to `MapBatches` incorrectly stating it doesn't modify row counts. (ray-project#35950) | | **June 27, 2023** | **Limit pushdown disabled** | Rule disabled because it incorrectly pushed limits past UDFs that modified row counts. (ray-project#36831) | | **April 28, 2025** | **Fusion restricted** | Added logic to stop fusing operators that modify row counts when the downstream has a batch size. `MapBatches` stayed fused only because of its incorrect property (ray-project#52570). | | **July 8, 2025** | **Limit pushdown re-enabled with special case** | Re-enabled with a special case to prevent pushing limits past `MapBatches`. ([ray-project#39486](ray-project#39486)) | | **Oct 24, 2025** | **Special case removed** | Special case removed, re-introducing the bug where limits are pushed past `MapBatches`. ([ray-project#57880](ray-project#57880)) | | **Feb 2, 2026** | **Property Fix** | Updated `MapBatches` to correctly report it modifies rows by default. This fixed the pushdown bug but broke fusion logic. ([PR ray-project#60448](ray-project#60448)) | | **Feb 4, 2026** | (This PR) | Add a special-case to preserve the historical `MapBatches` fusion behavior | --- <a href="https://cursor.com/dashboard?tab=bugbot">Cursor Bugbot</a> reviewed your changes and found no issues for commit d99e7b1 --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>

…ct#60756) This PR updates the operator fusion rule to fuse `MapBatches` even if they modify the row counts. The intention of this PR is to preserve the historical operator fusion behavior and avoid introducing regressions. For more details, see the timeline below. --- ### Timeline of Changes | Date | Event | Description | | :--- | :--- | :--- | | **June 8, 2023** | **Limit pushdown added** | Added limit pushdown and a property to `MapBatches` incorrectly stating it doesn't modify row counts. (ray-project#35950) | | **June 27, 2023** | **Limit pushdown disabled** | Rule disabled because it incorrectly pushed limits past UDFs that modified row counts. (ray-project#36831) | | **April 28, 2025** | **Fusion restricted** | Added logic to stop fusing operators that modify row counts when the downstream has a batch size. `MapBatches` stayed fused only because of its incorrect property (ray-project#52570). | | **July 8, 2025** | **Limit pushdown re-enabled with special case** | Re-enabled with a special case to prevent pushing limits past `MapBatches`. ([ray-project#39486](ray-project#39486)) | | **Oct 24, 2025** | **Special case removed** | Special case removed, re-introducing the bug where limits are pushed past `MapBatches`. ([ray-project#57880](ray-project#57880)) | | **Feb 2, 2026** | **Property Fix** | Updated `MapBatches` to correctly report it modifies rows by default. This fixed the pushdown bug but broke fusion logic. ([PR ray-project#60448](ray-project#60448)) | | **Feb 4, 2026** | (This PR) | Add a special-case to preserve the historical `MapBatches` fusion behavior | --- <a href="https://cursor.com/dashboard?tab=bugbot">Cursor Bugbot</a> reviewed your changes and found no issues for commit d99e7b1 --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu> Signed-off-by: Adel Nour <ans9868@nyu.edu>

…ct#60756) This PR updates the operator fusion rule to fuse `MapBatches` even if they modify the row counts. The intention of this PR is to preserve the historical operator fusion behavior and avoid introducing regressions. For more details, see the timeline below. --- ### Timeline of Changes | Date | Event | Description | | :--- | :--- | :--- | | **June 8, 2023** | **Limit pushdown added** | Added limit pushdown and a property to `MapBatches` incorrectly stating it doesn't modify row counts. (ray-project#35950) | | **June 27, 2023** | **Limit pushdown disabled** | Rule disabled because it incorrectly pushed limits past UDFs that modified row counts. (ray-project#36831) | | **April 28, 2025** | **Fusion restricted** | Added logic to stop fusing operators that modify row counts when the downstream has a batch size. `MapBatches` stayed fused only because of its incorrect property (ray-project#52570). | | **July 8, 2025** | **Limit pushdown re-enabled with special case** | Re-enabled with a special case to prevent pushing limits past `MapBatches`. ([ray-project#39486](ray-project#39486)) | | **Oct 24, 2025** | **Special case removed** | Special case removed, re-introducing the bug where limits are pushed past `MapBatches`. ([ray-project#57880](ray-project#57880)) | | **Feb 2, 2026** | **Property Fix** | Updated `MapBatches` to correctly report it modifies rows by default. This fixed the pushdown bug but broke fusion logic. ([PR ray-project#60448](ray-project#60448)) | | **Feb 4, 2026** | (This PR) | Add a special-case to preserve the historical `MapBatches` fusion behavior | --- <a href="https://cursor.com/dashboard?tab=bugbot">Cursor Bugbot</a> reviewed your changes and found no issues for commit d99e7b1 --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>

…ct#60756) This PR updates the operator fusion rule to fuse `MapBatches` even if they modify the row counts. The intention of this PR is to preserve the historical operator fusion behavior and avoid introducing regressions. For more details, see the timeline below. --- ### Timeline of Changes | Date | Event | Description | | :--- | :--- | :--- | | **June 8, 2023** | **Limit pushdown added** | Added limit pushdown and a property to `MapBatches` incorrectly stating it doesn't modify row counts. (ray-project#35950) | | **June 27, 2023** | **Limit pushdown disabled** | Rule disabled because it incorrectly pushed limits past UDFs that modified row counts. (ray-project#36831) | | **April 28, 2025** | **Fusion restricted** | Added logic to stop fusing operators that modify row counts when the downstream has a batch size. `MapBatches` stayed fused only because of its incorrect property (ray-project#52570). | | **July 8, 2025** | **Limit pushdown re-enabled with special case** | Re-enabled with a special case to prevent pushing limits past `MapBatches`. ([ray-project#39486](ray-project#39486)) | | **Oct 24, 2025** | **Special case removed** | Special case removed, re-introducing the bug where limits are pushed past `MapBatches`. ([ray-project#57880](ray-project#57880)) | | **Feb 2, 2026** | **Property Fix** | Updated `MapBatches` to correctly report it modifies rows by default. This fixed the pushdown bug but broke fusion logic. ([PR ray-project#60448](ray-project#60448)) | | **Feb 4, 2026** | (This PR) | Add a special-case to preserve the historical `MapBatches` fusion behavior | --- <a href="https://cursor.com/dashboard?tab=bugbot">Cursor Bugbot</a> reviewed your changes and found no issues for commit d99e7b1 --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu> Signed-off-by: peterxcli <peterxcli@gmail.com>

remove limit pushdown and related tests

2e6c74b

Signed-off-by: Scott Lee <sjl@anyscale.com>

scottjlee marked this pull request as ready for review June 26, 2023 23:28

scottjlee requested review from amogkam, bveeramani, c21, ericl, raulchen and scv119 as code owners June 26, 2023 23:28

scottjlee assigned raulchen and c21 Jun 26, 2023

raulchen approved these changes Jun 27, 2023

View reviewed changes

raulchen merged commit 466cb5b into ray-project:master Jun 27, 2023

akshay-anyscale mentioned this pull request Jul 21, 2023

Add service deployment instructions to stable diffusion template #37645

Closed

8 tasks

bveeramani mentioned this pull request Feb 4, 2026

[Data] Fuse MapBatches even if they modify the row count #60756

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Data] Disable Limit Pushdown rule in new execution plan optimizer#36831

[Data] Disable Limit Pushdown rule in new execution plan optimizer#36831
raulchen merged 1 commit intoray-project:masterfrom
scottjlee:disable-limit-pushdown

scottjlee commented Jun 26, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

scottjlee commented Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Related issue number

Checks

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

scottjlee commented Jun 26, 2023 •

edited

Loading