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

Rewrite CommonSubexprEliminate to avoid copies using TreeNode #10067

Closed
wants to merge 1 commit into from

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Apr 12, 2024

WIP -- still has many more copies to remove

Which issue does this PR close?

Part of #9637

related to #9873

Rationale for this change

Let's make planning faster by not copying so much in the optimizer using the new TreeNode API.

What changes are included in this PR?

  1. See PR subject

Note this pass still leaves a lot to be desired in terms of copying Exprs / making strings but it is much better now. Fixing up the additional copies is tracked with #9873

Are these changes tested?

Functional: existing tests
Performance: TBD

Are there any user-facing changes?

@peter-toth
Copy link
Contributor

@alamb, can I take this PR and rebase it on the latest main? It would be better to refactor CSE rule to use rewrite() before I can finish my #10473.

@alamb
Copy link
Contributor Author

alamb commented Jun 7, 2024

@alamb, can I take this PR and rebase it on the latest main? It would be better to refactor CSE rule to use rewrite() before I can finish my #10473.

Sure -- that would be great @peter-toth -- I actually had a secret plan to work on the CSE rewrite tomorrow (I swear!) as I will be traveling and on a plane ✈️ -- but if you would like to take it that would be great (I have several other things that I would love to work on as well)

Just let me know what you want to do

@peter-toth
Copy link
Contributor

Sure, please work on it if you have some time during the weekend as I can't do it earlier than next week.
Let me know how it goes and I can take this next week if you don't have time.

@alamb
Copy link
Contributor Author

alamb commented Jun 7, 2024

Sure, please work on it if you have some time during the weekend as I can't do it earlier than next week. Let me know how it goes and I can take this next week if you don't have time.

Will do -- thank you

@alamb
Copy link
Contributor Author

alamb commented Jun 8, 2024

Superceded by #10835, will continue work there

@alamb alamb closed this Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimizer Optimizer rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants