-
Notifications
You must be signed in to change notification settings - Fork 526
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
fix(streaming): avoid violating the assumption of U+ after U- in hash join & add checker #1811
Conversation
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
} | ||
|
||
#[async_trait] | ||
impl super::DebugExecutor for UpdateCheckExecutor { |
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.
Can we merge all debug executors into one? The backtrace would be disaster...
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.
Next time for sure!
also test failed |
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.
LGTM
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Codecov Report
@@ Coverage Diff @@
## main #1811 +/- ##
==========================================
+ Coverage 71.33% 71.35% +0.02%
==========================================
Files 605 606 +1
Lines 79168 79261 +93
==========================================
+ Hits 56475 56558 +83
- Misses 22693 22703 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
What's changed and what's your intention?
When there're multiple rows matched with a single
UpdateDelete
, the hash join executor will yield multiple continuousUpdateDelete
s, which violates the assumption of U+ after U- used byMerge
,Filter
and so on. One can simply reproduce this in tpch q17, q18 with rearranged chain in #1556 since it introduces more barriers than before.Checklist
Refer to a related PR or issue link (optional)
U+
U-
appears in pairs #1801