Skip to content

Conversation

samueleresca
Copy link
Member

@samueleresca samueleresca commented Oct 9, 2025

Which issue does this PR close?

Rationale for this change

The capacity calculation replaced with left.len() (assuming left.len() and right.len() are the same). As the with_capacity refers to the length of the views (or strings), not to the length of the bytes

Are these changes tested?

The function is already covered by tests.

Are there any user-facing changes?

No

@github-actions github-actions bot added the physical-expr Changes to the physical-expr crates label Oct 9, 2025
@samueleresca samueleresca marked this pull request as ready for review October 9, 2025 20:59
@ctsk
Copy link
Contributor

ctsk commented Oct 14, 2025

LGTM, that capacity initialization was wrong. It is unrelated to the overflow issue though, right?

Copy link
Contributor

@Jefffrey Jefffrey left a comment

Choose a reason for hiding this comment

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

Makes sense to me 👍

@samueleresca
Copy link
Member Author

LGTM, that capacity initialization was wrong. It is unrelated to the overflow issue though, right?

Yeah, this is unrelated to the overflow issue fix. AFAIU, it would just avoid allocating unneeded memory.

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

Labels

physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants