-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Fix](predicate pushdown) Common expression not acting on any slot should not be pushed down #25901
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
Conversation
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
be/src/vec/exec/scan/vscan_node.cpp
Outdated
| } | ||
| } | ||
|
|
||
| std::function<bool(const VExprSPtr&)> _conjunct_is_acting_on_a_slot = |
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.
这种的就不要写lambda了,直接定义一个函数吧
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.
那这应该定一一个 static bool VExpr::is_acting_on_a_slot() 吧?
be/src/vec/exec/scan/vscan_node.cpp
Outdated
| for (size_t i = 0; i < children_size; ++i) { | ||
| // If any child expr does not act on a column slot | ||
| // return false immediately. | ||
| if (!_conjunct_is_acting_on_a_slot(children[i])) { |
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.
这样做应该是不对的,应该是,只要有一个acting on slot,那么就应该是true;其他的是false
| // Two conjuncts. | ||
| // random() > 1 is executed by scan node, c1 > 0 is pushed down and executed by segment iterator | ||
| order_qt_6 """ | ||
| SELECT * FROM t_pushdown_common_expr WHERE random() > 1 AND c1 > 0 |
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.
寄一个select * from table where random() > 1 的测试case吧
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
yiguolei
left a comment
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
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
xinyiZzz
left a comment
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
…ould not be pushed down (apache#25901)
…ould not be pushed down (apache#25901)
…ould not be pushed down (apache#25901)
…ould not be pushed down (apache#25901) (apache#26215)
…ould not be pushed down (apache#25901)
Issue Number: close #25900
If conjunc does not act an any slot, do not push it down.