-
Notifications
You must be signed in to change notification settings - Fork 376
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
[GLUTEN-4029][CH]Improve multiIf #4030
Conversation
Run Gluten Clickhouse CI |
4e22c07
to
84e2615
Compare
Run Gluten Clickhouse CI |
Run Gluten Clickhouse CI |
const auto * if_node = toFunctionNode(actions_dag, "if", { | ||
or_condition_node, null_const_node, wrap_slice_node }); | ||
|
||
return if_ndoe; |
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.
convertTypeIfNeeded
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.
done
Run Gluten Clickhouse CI |
Run Gluten Clickhouse CI |
Run Gluten Clickhouse CI |
end_is_null_node, | ||
null_const_node, | ||
step_is_null_node, | ||
or_condition_node, |
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.
这里没改成if?
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.
流水线的case似乎没有用到这些函数,需要确认下正确性。
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.
multiif换成if有些case下有问题,参考#4042 有失败的case q86 q36.
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.
sequence 这个函数中multiIf 的条件不一样,不能将MulttiIf 改成 If, 只能减少MultiIf的判断条件; 这里的场景 和 #4042 中的不一样
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
What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
(Fixes: #4029)
How was this patch tested?
manual tests
测试数据
测试elt函数,测试SQL
select count(1) from test_tbl where elt(1, d) >1
, 表定义 test_tbl(d bigint) 数据量3000W, 测试3次PR改动前:8.936s, 9.921s, 9.661s
PR改动后:8.322s,7.845s,7.668s