Skip to content

fix: use try_shrink instead of shrink in try_resize - #20424

Merged
jonahgao merged 1 commit into
apache:mainfrom
ariel-miculas:use-try-shrink
Mar 4, 2026
Merged

fix: use try_shrink instead of shrink in try_resize#20424
jonahgao merged 1 commit into
apache:mainfrom
ariel-miculas:use-try-shrink

Conversation

@ariel-miculas

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

No issue

Rationale for this change

The try_ functions should return errors instead of panicking.

What changes are included in this PR?

use try_shrink instead of shrink

Are these changes tested?

No, try_shrink is already tested.

Are there any user-facing changes?

No

@github-actionsgithub-actionsBot added the execution Related to the execution crate label Feb 18, 2026
@xanderbailey

Copy link
Copy Markdown
Contributor

Do you have an example of when shrink fails in this case?

@ariel-miculas

Copy link
Copy Markdown
ContributorAuthor

I don't have any example of when this can happen, it's just that shrink Panics if capacity exceeds [Self::size], whereas try_shrink returns an error

@jonahgaojonahgao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks @ariel-miculas

@jonahgao
jonahgao added this pull request to the merge queueMar 4, 2026
Merged via the queue into apache:main with commit 2cbee47Mar 4, 2026
28 checks passed
alamb pushed a commit to alamb/datafusion that referenced this pull request Mar 12, 2026
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closesapache#123` indicates that this PR will close issue apache#123.
-->
No issue
## Rationale for this change
The `try_` functions should return errors instead of panicking.
## What changes are included in this PR?
use `try_shrink` instead of `shrink`
## Are these changes tested?
No, `try_shrink` is already tested.
## Are there any user-facing changes?
No
comphead pushed a commit that referenced this pull request Mar 12, 2026
…) (#20890)
- Part of #19692
This PR:
- Backports #20424 from
@ariel-miculas to the branch-53 line
Co-authored-by: Ariel Miculas-Trif <ariel.miculas@gmail.com>
de-bgunter pushed a commit to de-bgunter/datafusion that referenced this pull request Mar 24, 2026
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closesapache#123` indicates that this PR will close issue apache#123.
-->
No issue
## Rationale for this change
The `try_` functions should return errors instead of panicking.
## What changes are included in this PR?
use `try_shrink` instead of `shrink`
## Are these changes tested?
No, `try_shrink` is already tested.
## Are there any user-facing changes?
No
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

executionRelated to the execution crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ariel-miculas@xanderbailey@jonahgao