You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "seawinde (via GitHub)" <gi...@apache.org> on 2023/12/28 12:52:26 UTC
[PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
seawinde opened a new pull request, #29256:
URL: https://github.com/apache/doris/pull/29256
## Proposed changes
when aggregate function roll up, we should check the qury and mv function argument is equal
such as mv def and query sql as following, it should not rewrite success, because the bitmap_union_basic field augument is
not equal to the `count(distinct case when o_shippriority > 10 and o_orderkey IN (1, 3) then o_custkey else null end)` field in query
mv def:
> select l_shipdate, o_orderdate, l_partkey, l_suppkey,
sum(o_totalprice) as sum_total,
max(o_totalprice) as max_total,
min(o_totalprice) as min_total,
count(*) as count_all,
bitmap_union(to_bitmap(case when o_shippriority > 1 and o_orderkey IN (1, 3) then o_custkey else null end)) as bitmap_union_basic
from lineitem
left join orders on lineitem.l_orderkey = orders.o_orderkey and l_shipdate = o_orderdate
group by
l_shipdate,
o_orderdate,
l_partkey,
l_suppkey;
query sql:
> select t1.l_partkey, t1.l_suppkey, o_orderdate,
sum(o_totalprice),
max(o_totalprice),
min(o_totalprice),
count(*),
count(distinct case when o_shippriority > 10 and o_orderkey IN (1, 3) then o_custkey else null end)
from (select * from lineitem where l_shipdate = '2023-12-11') t1
left join orders on t1.l_orderkey = orders.o_orderkey and t1.l_shipdate = o_orderdate
group by
o_orderdate,
l_partkey,
l_suppkey;
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1871655229
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 48.21 seconds
stream load tsv: 566 seconds loaded 74807831229 Bytes, about 126 MB/s
stream load json: 19 seconds loaded 2358488459 Bytes, about 118 MB/s
stream load orc: 67 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.9 seconds inserted 10000000 Rows, about 346K ops/s
storage size: 17188051991 Bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "seawinde (via GitHub)" <gi...@apache.org>.
seawinde commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1874115079
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1874170530
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 46.8 seconds
stream load tsv: 577 seconds loaded 74807831229 Bytes, about 123 MB/s
stream load json: 19 seconds loaded 2358488459 Bytes, about 118 MB/s
stream load orc: 66 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.3 seconds inserted 10000000 Rows, about 353K ops/s
storage size: 17183722708 Bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "seawinde (via GitHub)" <gi...@apache.org>.
seawinde commented on code in PR #29256:
URL: https://github.com/apache/doris/pull/29256#discussion_r1439305174
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -267,7 +273,7 @@ private Function rollup(AggregateFunction queryFunction,
if (!(mvExprShuttled instanceof Function)) {
continue;
}
- if (isAggregateFunctionEquivalent(queryFunction, (Function) mvExprShuttled)) {
+ if (isAggregateFunctionEquivalent(queryFunction, queryFunctionShuttled, (Function) mvExprShuttled)) {
Review Comment:
OK,I will add more comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "seawinde (via GitHub)" <gi...@apache.org>.
seawinde commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1874048749
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1874773003
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1875041015
run pipelinex_p0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1874156826
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
```
Tpch sf100 test result on commit eefc3259260b7fab26b1cf0b5b49f5418dfb6f62, data reload: false
run tpch-sf100 query with default conf and session variables
q1 5505 5167 5098 5098
q2 405 160 157 157
q3 1465 1181 1078 1078
q4 1093 902 799 799
q5 3086 3109 3104 3104
q6 230 143 132 132
q7 994 537 577 537
q8 2153 2180 2165 2165
q9 6656 6663 6647 6647
q10 3204 3172 3130 3130
q11 343 213 217 213
q12 378 241 233 233
q13 4371 3587 3627 3587
q14 266 224 209 209
q15 611 535 543 535
q16 461 391 395 391
q17 1035 547 561 547
q18 7078 6706 6734 6706
q19 1634 1523 1550 1523
q20 603 348 361 348
q21 2895 2457 2488 2457
q22 398 316 338 316
Total cold run time: 44864 ms
Total hot run time: 39912 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 5056 5065 5183 5065
q2 336 241 246 241
q3 3353 3284 3313 3284
q4 2148 2035 2013 2013
q5 5932 5906 5905 5905
q6 226 126 125 125
q7 2379 1932 1957 1932
q8 3565 3640 3641 3640
q9 9091 8999 8985 8985
q10 3837 3901 3899 3899
q11 571 497 495 495
q12 805 641 621 621
q13 3866 3162 3170 3162
q14 289 301 275 275
q15 603 554 557 554
q16 541 496 567 496
q17 2012 1846 1799 1799
q18 8691 8346 8305 8305
q19 1752 1679 1656 1656
q20 2311 1978 1959 1959
q21 5771 5251 5330 5251
q22 576 497 491 491
Total cold run time: 63711 ms
Total hot run time: 60153 ms
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1874881728
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1871651030
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
```
Tpch sf100 test result on commit b30f7d69878ea254adc50b2a240794c78ac2daab, data reload: false
run tpch-sf100 query with default conf and session variables
q1 5522 5168 5116 5116
q2 390 165 159 159
q3 1496 1150 1162 1150
q4 1084 815 799 799
q5 3145 3098 2987 2987
q6 231 139 137 137
q7 963 567 546 546
q8 2156 2200 2265 2200
q9 6701 6640 6640 6640
q10 3177 3199 3183 3183
q11 343 225 229 225
q12 391 239 241 239
q13 4392 3639 3623 3623
q14 259 222 215 215
q15 637 573 550 550
q16 452 400 406 400
q17 1046 555 691 555
q18 7050 6783 6781 6781
q19 1638 1517 1595 1517
q20 570 357 580 357
q21 2911 2482 2495 2482
q22 399 311 316 311
Total cold run time: 44953 ms
Total hot run time: 40172 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 5118 5083 5279 5083
q2 346 269 272 269
q3 3374 3323 3322 3322
q4 2158 2018 2036 2018
q5 5955 5930 5942 5930
q6 240 134 130 130
q7 2376 1947 1934 1934
q8 3561 3654 3632 3632
q9 9003 8900 8998 8900
q10 3908 3924 3961 3924
q11 579 493 475 475
q12 812 632 676 632
q13 3885 3192 3194 3192
q14 306 262 287 262
q15 622 555 554 554
q16 561 547 565 547
q17 2002 1808 1833 1808
q18 8773 8404 8399 8399
q19 1771 1731 1693 1693
q20 2304 1998 1998 1998
q21 5718 5257 5261 5257
q22 551 542 505 505
Total cold run time: 63923 ms
Total hot run time: 60464 ms
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "seawinde (via GitHub)" <gi...@apache.org>.
seawinde commented on PR #29256:
URL: https://github.com/apache/doris/pull/29256#issuecomment-1871640612
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on code in PR #29256:
URL: https://github.com/apache/doris/pull/29256#discussion_r1439181118
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TreeNode.java:
##########
@@ -241,6 +242,19 @@ default <T> Set<T> collectToSet(Predicate<TreeNode<NODE_TYPE>> predicate) {
return (Set<T>) result.build();
}
+ /**
+ * Collect the nodes that satisfied the predicate firstly.
+ */
+ default <T> List<T> collectFirst(Predicate<TreeNode<NODE_TYPE>> predicate) {
+ List<TreeNode<NODE_TYPE>> result = new ArrayList<>();
+ foreach(node -> {
Review Comment:
maybe we could impl it more efficiently
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -60,15 +63,18 @@
*/
public abstract class AbstractMaterializedViewAggregateRule extends AbstractMaterializedViewRule {
- protected static final Map<Expression, Expression>
- AGGREGATE_ROLL_UP_EQUIVALENT_FUNCTION_MAP = new HashMap<>();
+ // we only support roll up function which has only one argument currently
+ protected static final List<Pair<Expression, Expression>>
+ AGGREGATE_ROLL_UP_EQUIVALENT_FUNCTION_LIST = new ArrayList<>();
Review Comment:
`map<Expression, map<Expression, Expression>>` maybe better?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -267,7 +273,7 @@ private Function rollup(AggregateFunction queryFunction,
if (!(mvExprShuttled instanceof Function)) {
continue;
}
- if (isAggregateFunctionEquivalent(queryFunction, (Function) mvExprShuttled)) {
+ if (isAggregateFunctionEquivalent(queryFunction, queryFunctionShuttled, (Function) mvExprShuttled)) {
Review Comment:
add more comment to explain each parameters of function `rollup`, `isAggregateFunctionEquivalent` and `extractViewArguments`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow merged PR #29256:
URL: https://github.com/apache/doris/pull/29256
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](nereids) fix aggregate function roll up when expression arguments is not equals [doris]
Posted by "seawinde (via GitHub)" <gi...@apache.org>.
seawinde commented on code in PR #29256:
URL: https://github.com/apache/doris/pull/29256#discussion_r1439304800
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java:
##########
@@ -60,15 +63,18 @@
*/
public abstract class AbstractMaterializedViewAggregateRule extends AbstractMaterializedViewRule {
- protected static final Map<Expression, Expression>
- AGGREGATE_ROLL_UP_EQUIVALENT_FUNCTION_MAP = new HashMap<>();
+ // we only support roll up function which has only one argument currently
+ protected static final List<Pair<Expression, Expression>>
+ AGGREGATE_ROLL_UP_EQUIVALENT_FUNCTION_LIST = new ArrayList<>();
Review Comment:
Yeah you are right, I decide to use `MultiMap`, it's almost same to the `map<Expression, map<Expression, Expression>> `
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org