Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor: Avoid an unecessary contruction in map_children some extra plan construction #5761

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Mar 28, 2023

Which issue does this PR close?

Related to #5630

Rationale for this change

As suggested by @mingmwang #5630 (comment) we can save a copy here

Are these changes tested?

Covered by existing tests

Are there any user-facing changes?

@github-actions github-actions bot added the logical-expr Logical plan and expressions label Mar 28, 2023
@alamb alamb requested a review from yahoNanJing March 28, 2023 11:39
@alamb alamb changed the title Minor: Avoid some extra plan construction Minor: Avoid an unecessary contruction in map_children some extra plan construction Mar 28, 2023
@mingmwang
Copy link
Contributor

LGTM.
An empty iterator will return false for the iter().any(), this is exptected.

.collect::<Result<Vec<_>>>()?;

// if any changes made, make a new child
if old_children
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alamb alamb merged commit c8a3d58 into apache:main Mar 28, 2023
@yahoNanJing
Copy link
Contributor

LGTM

@alamb alamb deleted the alamb/cleanups4 branch March 30, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logical-expr Logical plan and expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants