Skip to content

Remove Expr clones from SortExprs - #13258

Merged
crepererum merged 2 commits into
apache:mainfrom
peter-toth:remove-sort-expr-clones
Nov 5, 2024
Merged

Remove Expr clones from SortExprs#13258
crepererum merged 2 commits into
apache:mainfrom
peter-toth:remove-sort-expr-clones

Conversation

@peter-toth

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Follow-up of #12177, part of #12193.

Rationale for this change

Expr clones can be costly, so let's avoid them if we can.

What changes are included in this PR?

This is a minor follow-up of #12177 to remove some Expr clones.

Are these changes tested?

Yes, with exiting UTs.

Are there any user-facing changes?

No.

@github-actionsgithub-actionsBot added logical-expr Logical plan and expressions optimizer Optimizer rules labels Nov 5, 2024
@peter-toth
peter-tothforce-pushed the remove-sort-expr-clones branch from ed51689 to 3b598ecCompareNovember 5, 2024 09:56
@github-actionsgithub-actionsBot added the core Core DataFusion crate label Nov 5, 2024
@peter-toth
peter-tothforce-pushed the remove-sort-expr-clones branch from 3b598ec to 47df15aCompareNovember 5, 2024 10:15
@peter-toth

Copy link
Copy Markdown
ContributorAuthor

cc @findepi, @alamb, @crepererum

@alambalamb left a comment

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.

Thank you @peter-toth -- this looks very nice to me

Comment threaddatafusion/expr/src/expr.rs
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

@crepererumcrepererum left a comment

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.

thank you

@crepererum
crepererum merged commit 19779d8 into apache:mainNov 5, 2024
@peter-toth

Copy link
Copy Markdown
ContributorAuthor

Thanks @alamb and @crepererum for the review!

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

Labels

coreCore DataFusion cratelogical-exprLogical plan and expressionsoptimizerOptimizer rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@peter-toth@alamb@crepererum