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

Avoid copies in TypeCoercion via TreeNode API #10039

Closed
wants to merge 3 commits into from

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Apr 10, 2024

Draft as it has a failure in tpchds planning for some reason

Which issue does this PR close?

Closes #10210

Part of #9637 -- let's make DataFusion planning faster by not copying so much

Rationale for this change

Now that we have the nice TreeNode API thanks to #8913 and @peter-toth let's use it to both simplify the code and avoid copies

What changes are included in this PR?

  1. Avoid copies in TypeCoercion via TreeNode API

Are these changes tested?

Existing CI

Are there any user-facing changes?

@alamb alamb marked this pull request as draft April 10, 2024 22:50
@github-actions github-actions bot added optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) labels Apr 10, 2024
@alamb alamb force-pushed the alamb/less_analyzer_copy2 branch from 62c8a16 to afb07c6 Compare April 12, 2024 18:36
@github-actions github-actions bot removed the sqllogictest SQL Logic Tests (.slt) label Apr 12, 2024
@alamb alamb force-pushed the alamb/less_analyzer_copy2 branch from afb07c6 to f2e2c3b Compare April 23, 2024 18:55
@github-actions github-actions bot added the logical-expr Logical plan and expressions label Apr 23, 2024
@alamb
Copy link
Contributor Author

alamb commented Apr 23, 2024

An update here is twofold:

  1. There are some very subtle semantics going on
  2. As this pass actually changes the types of the plan (on purpose) we need some way to recalculate the schemas (which is what Plan::with_new_exprs does, but it also requires a copy of the inputs and exprs, which is not cool

I am still putzing with how to make this better

@alamb
Copy link
Contributor Author

alamb commented May 2, 2024

superceded by #10356

@alamb alamb closed this May 2, 2024
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 optimizer Optimizer rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop copying LogicalPlan and Exprs in TypeCoercion
1 participant