simplify the between expr during logical plan optimization
#3404
Conversation
| " CrossJoin: [l_partkey:Int64, l_quantity:Float64, p_partkey:Int64, p_brand:Utf8, p_size:Int32]", | ||
| " TableScan: lineitem projection=[l_partkey, l_quantity] [l_partkey:Int64, l_quantity:Float64]", | ||
| " TableScan: part projection=[p_partkey, p_brand, p_size] [p_partkey:Int64, p_brand:Utf8, p_size:Int32]", | ||
| " Filter: #part.p_size >= Int32(1) [p_partkey:Int64, p_brand:Utf8, p_size:Int32]", |
There was a problem hiding this comment.
This is a great optimization. Next goal is to get it to see that part.p_size <= 15 as well :D
There was a problem hiding this comment.
Oof, that would be excellent. For another PR if someone doesn't beat me to it :)
There was a problem hiding this comment.
Or maybe sooner .. looks like the order of the projections changes between runs, which is causing the test failure here.
Codecov Report
@@ Coverage Diff @@
## master #3404 +/- ##
==========================================
- Coverage 85.58% 85.49% -0.10%
==========================================
Files 296 296
Lines 54252 54328 +76
==========================================
+ Hits 46432 46446 +14
- Misses 7820 7882 +62
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Putting in draft until I can figure out this bug that's triggered by this change. :/ |
Commonly, a hash table or hash set generates these kind of random results. Maybe an optimization rule uses this, where this could be fixed (I e. producing in order of appearance or sorting the results). |
|
@Dandandan good advice, thank you :) |
xudong963
left a comment
xudong963
left a comment
There was a problem hiding this comment.
LGTM, thanks @kmitchener . I believe the optimization can bring more potential benefits 👍
|
Benchmark runs are scheduled for baseline = eaf1d46 and contender = 73447b5. 73447b5 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
5 participants
Which issue does this PR close?
Closes #3402 .
Rationale for this change
Allows the plan to be further optimized and simplified, resulting in better plans.
What changes are included in this PR?
Are there any user-facing changes?