Skip to content

[Data] Set udf-modifying-row-count default to false#58264

Merged
alexeykudinkin merged 1 commit intoray-project:masterfrom
owenowenisme:data/set-udf-modifying-row-count-to-true
Oct 29, 2025
Merged

[Data] Set udf-modifying-row-count default to false#58264
alexeykudinkin merged 1 commit intoray-project:masterfrom
owenowenisme:data/set-udf-modifying-row-count-to-true

Conversation

@owenowenisme
Copy link
Copy Markdown
Member

@owenowenisme owenowenisme commented Oct 29, 2025

Description

We want to keep the limit pushdown as default so we should set udf_modifying_row_count default to false as default.

Related issues

Additional information

@owenowenisme owenowenisme added the go add ONLY when ready to merge, run all tests label Oct 29, 2025
Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
@owenowenisme owenowenisme force-pushed the data/set-udf-modifying-row-count-to-true branch from 494d193 to c0aa88c Compare October 29, 2025 03:33
@owenowenisme owenowenisme marked this pull request as ready for review October 29, 2025 08:25
@owenowenisme owenowenisme requested a review from a team as a code owner October 29, 2025 08:25
isinstance(current_op, AbstractOneToOne)
and not current_op.can_modify_num_rows()
):
if isinstance(current_op, AbstractMap):
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

One thing worth mention is that we shouldnt push through those _min_rows_per_bundled_input is < limit or it will break some behavior, for example, the schema api 's limit(1) shouldn't push through if it is used with map_batches(batch_size=2)

@ray-gardener ray-gardener bot added the data Ray Data-related issues label Oct 29, 2025
@alexeykudinkin alexeykudinkin enabled auto-merge (squash) October 29, 2025 23:11
if min_rows is not None and min_rows > limit_op._limit:
# Avoid pushing the limit past batch-based maps that require more
# rows than the limit to produce stable outputs (e.g. schema).
break
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.

Let's add a log statement here to be able to trace this from the logs

@alexeykudinkin alexeykudinkin merged commit ef1395b into ray-project:master Oct 29, 2025
7 checks passed
alexeykudinkin pushed a commit that referenced this pull request Oct 30, 2025
…imit` (#58303)

## Description

## Related issues

Fix comment
#58264 (comment)
## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
YoussefEssDS pushed a commit to YoussefEssDS/ray that referenced this pull request Nov 8, 2025
)

## Description

We want to keep the limit pushdown as default so we should set
`udf_modifying_row_count` default to false as default.

## Related issues

## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
YoussefEssDS pushed a commit to YoussefEssDS/ray that referenced this pull request Nov 8, 2025
…imit` (ray-project#58303)

## Description

## Related issues

Fix comment
ray-project#58264 (comment)
## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
)

## Description

We want to keep the limit pushdown as default so we should set
`udf_modifying_row_count` default to false as default.

## Related issues

## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
…imit` (ray-project#58303)

## Description

## Related issues

Fix comment
ray-project#58264 (comment)
## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
Aydin-ab pushed a commit to Aydin-ab/ray-aydin that referenced this pull request Nov 19, 2025
)

## Description

We want to keep the limit pushdown as default so we should set
`udf_modifying_row_count` default to false as default.

## Related issues

## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Aydin-ab pushed a commit to Aydin-ab/ray-aydin that referenced this pull request Nov 19, 2025
…imit` (ray-project#58303)

## Description

## Related issues

Fix comment
ray-project#58264 (comment)
## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Future-Outlier pushed a commit to Future-Outlier/ray that referenced this pull request Dec 7, 2025
)

## Description

We want to keep the limit pushdown as default so we should set
`udf_modifying_row_count` default to false as default.

## Related issues

## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
Signed-off-by: Future-Outlier <eric901201@gmail.com>
Future-Outlier pushed a commit to Future-Outlier/ray that referenced this pull request Dec 7, 2025
…imit` (ray-project#58303)

## Description

## Related issues

Fix comment
ray-project#58264 (comment)
## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
Signed-off-by: Future-Outlier <eric901201@gmail.com>
peterxcli pushed a commit to peterxcli/ray that referenced this pull request Feb 25, 2026
)

## Description

We want to keep the limit pushdown as default so we should set
`udf_modifying_row_count` default to false as default.

## Related issues

## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
Signed-off-by: peterxcli <peterxcli@gmail.com>
peterxcli pushed a commit to peterxcli/ray that referenced this pull request Feb 25, 2026
…imit` (ray-project#58303)

## Description

## Related issues

Fix comment
ray-project#58264 (comment)
## Additional information

Signed-off-by: You-Cheng Lin <mses010108@gmail.com>
Signed-off-by: peterxcli <peterxcli@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data Ray Data-related issues go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants