Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[fix](window_function) min/max/sum/avg should be always nullable #27104

Merged
merged 1 commit into from
Nov 18, 2023
Merged

[fix](window_function) min/max/sum/avg should be always nullable #27104

merged 1 commit into from
Nov 18, 2023

Conversation

mrhhsg
Copy link
Member

@mrhhsg mrhhsg commented Nov 16, 2023

Proposed changes

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@mrhhsg mrhhsg force-pushed the window_always_nullable branch 2 times, most recently from 20521ba to e7252ff Compare November 16, 2023 08:08
@mrhhsg
Copy link
Member Author

mrhhsg commented Nov 16, 2023

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@mrhhsg mrhhsg force-pushed the window_always_nullable branch from e7252ff to 9b95e15 Compare November 16, 2023 08:27
@mrhhsg
Copy link
Member Author

mrhhsg commented Nov 16, 2023

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.77% (8413/22883)
Line Coverage: 29.29% (68449/233671)
Region Coverage: 27.88% (35373/126883)
Branch Coverage: 24.63% (18071/73364)
Coverage Report: http://coverage.selectdb-in.cc/coverage/e7252ff5ddd673508a1c003fa58aada63e8a76da_e7252ff5ddd673508a1c003fa58aada63e8a76da/report/index.html

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.79 seconds
stream load tsv: 578 seconds loaded 74807831229 Bytes, about 123 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 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: 17102913617 Bytes

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.77% (8413/22883)
Line Coverage: 29.29% (68441/233671)
Region Coverage: 27.87% (35367/126883)
Branch Coverage: 24.63% (18071/73364)
Coverage Report: http://coverage.selectdb-in.cc/coverage/9b95e15bf7286cb8e966fd79369f6b48496f140c_9b95e15bf7286cb8e966fd79369f6b48496f140c/report/index.html

@mrhhsg mrhhsg force-pushed the window_always_nullable branch from 9b95e15 to 905b223 Compare November 16, 2023 10:00
@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' 
Tpch sf100 test result on commit 9b95e15bf7286cb8e966fd79369f6b48496f140c, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4926	4640	4664	4640
q2	357	148	161	148
q3	2022	1920	1920	1920
q4	1376	1257	1257	1257
q5	3942	3967	4023	3967
q6	248	129	131	129
q7	1357	880	895	880
q8	2760	2791	2779	2779
q9	9880	9614	9588	9588
q10	3441	3533	3540	3533
q11	384	248	256	248
q12	438	308	291	291
q13	4558	3796	3775	3775
q14	343	292	285	285
q15	580	539	536	536
q16	662	584	588	584
q17	1134	968	927	927
q18	7804	7281	7362	7281
q19	1668	1678	1680	1678
q20	540	334	289	289
q21	4359	3939	3977	3939
q22	477	367	369	367
Total cold run time: 53256 ms
Total hot run time: 49041 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4612	4561	4591	4561
q2	349	219	247	219
q3	4016	4007	4005	4005
q4	2711	2700	2691	2691
q5	9648	9612	9697	9612
q6	243	124	123	123
q7	2607	2293	2282	2282
q8	4425	4459	4396	4396
q9	13157	13130	13169	13130
q10	4072	4206	4150	4150
q11	769	633	644	633
q12	977	807	818	807
q13	4298	3585	3543	3543
q14	377	359	362	359
q15	573	515	516	515
q16	752	678	660	660
q17	3923	3853	3869	3853
q18	9491	9000	9155	9000
q19	1800	1790	1781	1781
q20	2389	2061	2041	2041
q21	8845	8776	8808	8776
q22	866	798	838	798
Total cold run time: 80900 ms
Total hot run time: 77935 ms

@mrhhsg
Copy link
Member Author

mrhhsg commented Nov 16, 2023

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.79 seconds
stream load tsv: 580 seconds loaded 74807831229 Bytes, about 123 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.6 seconds inserted 10000000 Rows, about 349K ops/s
storage size: 17099000731 Bytes

@starocean999 starocean999 mentioned this pull request Nov 16, 2023
Closed
@mrhhsg mrhhsg force-pushed the window_always_nullable branch from 905b223 to df857b9 Compare November 16, 2023 10:32
@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.77% (8413/22883)
Line Coverage: 29.29% (68447/233687)
Region Coverage: 27.88% (35371/126891)
Branch Coverage: 24.63% (18072/73372)
Coverage Report: http://coverage.selectdb-in.cc/coverage/905b223fda57a9b09f7eb4eb3bb7441f9761db4c_905b223fda57a9b09f7eb4eb3bb7441f9761db4c/report/index.html

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 46.58 seconds
stream load tsv: 582 seconds loaded 74807831229 Bytes, about 122 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 28.8 seconds inserted 10000000 Rows, about 347K ops/s
storage size: 17100909100 Bytes

@mrhhsg mrhhsg force-pushed the window_always_nullable branch from df857b9 to 25c8d3b Compare November 16, 2023 14:03
@mrhhsg
Copy link
Member Author

mrhhsg commented Nov 16, 2023

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.76% (8411/22883)
Line Coverage: 29.27% (68411/233687)
Region Coverage: 27.86% (35358/126893)
Branch Coverage: 24.62% (18067/73374)
Coverage Report: http://coverage.selectdb-in.cc/coverage/25c8d3b0f1ab21d54413b24998316abbf217fcf3_25c8d3b0f1ab21d54413b24998316abbf217fcf3/report/index.html

@mrhhsg @starocean999
[fix](window_function) min/max/sum/avg should be always nullable
2aa0a69 
Co-authored-by: starocean999 <40539150+starocean999@users.noreply.github.com>
@mrhhsg mrhhsg force-pushed the window_always_nullable branch from 25c8d3b to 2aa0a69 Compare November 16, 2023 14:41
@mrhhsg
Copy link
Member Author

mrhhsg commented Nov 16, 2023

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 47.42 seconds
stream load tsv: 588 seconds loaded 74807831229 Bytes, about 121 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 29.5 seconds inserted 10000000 Rows, about 338K ops/s
storage size: 17099473669 Bytes

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' 
Tpch sf100 test result on commit 2aa0a696b8c392f7dd862563367161725a7f44b7, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4897	4631	4668	4631
q2	358	159	164	159
q3	2016	1893	1887	1887
q4	1388	1290	1232	1232
q5	3999	3933	4031	3933
q6	252	126	133	126
q7	1410	861	882	861
q8	2758	2798	2772	2772
q9	9821	9819	9547	9547
q10	3449	3505	3514	3505
q11	381	252	243	243
q12	434	289	294	289
q13	4535	3812	3797	3797
q14	309	278	296	278
q15	570	528	527	527
q16	678	588	584	584
q17	1141	960	963	960
q18	7853	7440	7411	7411
q19	1673	1694	1671	1671
q20	552	313	301	301
q21	4422	3943	3999	3943
q22	476	365	368	365
Total cold run time: 53372 ms
Total hot run time: 49022 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4584	4589	4584	4584
q2	337	232	245	232
q3	4009	4008	3991	3991
q4	2722	2692	2698	2692
q5	9861	9882	9830	9830
q6	248	121	125	121
q7	2599	2278	2242	2242
q8	4422	4506	4453	4453
q9	13315	13201	13179	13179
q10	4070	4179	4215	4179
q11	764	632	630	630
q12	977	820	830	820
q13	4332	3618	3603	3603
q14	373	355	343	343
q15	587	527	525	525
q16	734	700	681	681
q17	3900	3836	3860	3836
q18	9592	8871	9071	8871
q19	1815	1753	1763	1753
q20	2443	2087	2037	2037
q21	8941	8634	8734	8634
q22	893	827	787	787
Total cold run time: 81518 ms
Total hot run time: 78023 ms

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.68 seconds
stream load tsv: 583 seconds loaded 74807831229 Bytes, about 122 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.2 seconds inserted 10000000 Rows, about 354K ops/s
storage size: 17101298617 Bytes

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.76% (8412/22883)
Line Coverage: 29.28% (68421/233687)
Region Coverage: 27.87% (35368/126893)
Branch Coverage: 24.63% (18071/73374)
Coverage Report: http://coverage.selectdb-in.cc/coverage/2aa0a696b8c392f7dd862563367161725a7f44b7_2aa0a696b8c392f7dd862563367161725a7f44b7/report/index.html

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 17, 2023
@github-actions GitHub Actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

yiguolei
yiguolei approved these changes Nov 18, 2023
View reviewed changes
Copy link
Contributor

@yiguolei yiguolei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yiguolei yiguolei merged commit b42828c into apache:master Nov 18, 2023
superdiaodiao pushed a commit to superdiaodiao/doris that referenced this pull request Nov 21, 2023
@mrhhsg @starocean999 @superdiaodiao
[fix](window_function) min/max/sum/avg should be always nullable (apa…
b8d56fb 
…che#27104)

Co-authored-by: starocean999 <40539150+starocean999@users.noreply.github.com>
seawinde pushed a commit to seawinde/doris that referenced this pull request Nov 28, 2023
@mrhhsg @starocean999 @seawinde
[fix](window_function) min/max/sum/avg should be always nullable (apa…
6e3dc5e 
…che#27104)

Co-authored-by: starocean999 <40539150+starocean999@users.noreply.github.com>
XuJianxu pushed a commit to XuJianxu/doris that referenced this pull request Dec 14, 2023
@mrhhsg @starocean999
[fix](window_function) min/max/sum/avg should be always nullable (apa…
68bebaa 
…che#27104)

Co-authored-by: starocean999 <40539150+starocean999@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yiguolei yiguolei yiguolei approved these changes

@zhangstar333 zhangstar333 zhangstar333 approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.0.4 need_more_review reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mrhhsg @doris-robot @yiguolei @zhangstar333 @xiaokang @wm1581066