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: Add additional documentation to CommonSubexprEliminate #9959

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Apr 5, 2024

Which issue does this PR close?

Related to #9637 and #3058

Rationale for this change

As part of #9637 I plan to rework CommonSubexprEliminate a bit to stop copying as much

I started looking at this code, and I wanted to document how it worked more clearly (mostly for my future self, but I think it will be more generally useful)

What changes are included in this PR?

  1. Add doc comments in CommonSubexprEliminate

Are these changes tested?

Doc CI tests

Are there any user-facing changes?

Only some minor docstring changes. No functional changes intended

@alamb alamb added the documentation Improvements or additions to documentation label Apr 5, 2024
@github-actions github-actions bot added optimizer Optimizer rules and removed documentation Improvements or additions to documentation labels Apr 5, 2024
@alamb
Copy link
Contributor Author

alamb commented Apr 5, 2024

FYI @wiedld and @waynexia I think you have worked on this code at some point so may be interested in this PR

@alamb alamb changed the title Minor: Add documentation to CommonSubexprEliminate Minor: Add additional documentation to CommonSubexprEliminate Apr 5, 2024
Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

Nice thanks @alamb I was thinking if we need to have an optimizer rules doc with just list of rules and description what every rule is supposed to do

@alamb
Copy link
Contributor Author

alamb commented Apr 5, 2024

Nice thanks @alamb I was thinking if we need to have an optimizer rules doc with just list of rules and description what every rule is supposed to do

I agree -- this would help. I'll spend a few minutes updating our docs with that info

Update: #9967

@alamb alamb merged commit da555fb into apache:main Apr 5, 2024
25 checks passed
@alamb alamb deleted the alamb/cse_docs branch April 5, 2024 19:24
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