-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 comments about rewrite_disjunctive_predicate #3351
Conversation
/// ) | ||
/// ``` | ||
/// | ||
#[derive(Default)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just moved this code to the top of the module so that the comments describing what it does can be attached to the structure as well as being at the top of the file, for better discoverability
No change in behavior is intended
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thank you @alamb
Codecov Report
@@ Coverage Diff @@
## master #3351 +/- ##
=======================================
Coverage 85.48% 85.48%
=======================================
Files 294 294
Lines 54120 54120
=======================================
Hits 46265 46265
Misses 7855 7855
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Benchmark runs are scheduled for baseline = 0bbf127 and contender = d7fa064. d7fa064 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
re #217 and #78
Rationale for this change
While was working on #3334, @DhamoPS noticed that TPCH Q19 was not being rewritten, despite the code in https://github.com/apache/arrow-datafusion/blob/master/datafusion/optimizer/src/rewrite_disjunctive_predicate.rs from @xudong963
However, I think it was not very clear what the intent of the rewrite pass was so I figured that some docstrings would help
What changes are included in this PR?
Add docstrings to optimizer pass, and move structure up so the comments are more discoverable
Are there any user-facing changes?
No