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

[Bug](compatibility) fix stddev_samp function coredump when upgrade #40438

Merged
merged 1 commit into from
Sep 6, 2024
Merged

[Bug](compatibility) fix stddev_samp function coredump when upgrade #40438

merged 1 commit into from
Sep 6, 2024

Conversation

zhangstar333
Copy link
Contributor

@zhangstar333 zhangstar333 commented Sep 5, 2024 •
edited
Loading

Proposed changes

            if (IAggregateFunction::version < AGG_FUNCTION_NULLABLE) {
                return make_nullable(Data::get_return_type());
            } else {
                return Data::get_return_type();
            }

before check two version , and then get return type,
but now in branch-21, the IAggregateFunction::version have update to 5,
and also AGG_FUNCTION_NEW=5
so the check will not get right return type.

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@zhangstar333
Copy link
Contributor Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 5, 2024

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

@doris-robot
Copy link

TPC-H: Total hot run time: 38129 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 4563a2a7e047f711a3e431bc705a1448a40c858d, data reload: false

------ Round 1 ----------------------------------
q1	18170	4459	4393	4393
q2	2819	189	185	185
q3	11152	1120	1035	1035
q4	11011	702	826	702
q5	7965	2890	2789	2789
q6	233	138	140	138
q7	960	609	588	588
q8	9351	2046	2055	2046
q9	7164	6557	6532	6532
q10	6984	2209	2252	2209
q11	452	233	241	233
q12	392	222	227	222
q13	17889	3093	3076	3076
q14	270	230	240	230
q15	545	504	477	477
q16	594	503	517	503
q17	961	768	687	687
q18	7372	6817	6962	6817
q19	1395	1029	1074	1029
q20	679	327	342	327
q21	3905	3061	2901	2901
q22	1126	1010	1013	1010
Total cold run time: 111389 ms
Total hot run time: 38129 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4334	4312	4323	4312
q2	388	284	279	279
q3	2917	2661	2708	2661
q4	1987	1619	1619	1619
q5	5398	5412	5400	5400
q6	219	133	133	133
q7	2101	1729	1762	1729
q8	3237	3321	3364	3321
q9	8469	8492	8355	8355
q10	3459	3192	3210	3192
q11	603	524	520	520
q12	786	637	612	612
q13	10334	3103	3087	3087
q14	307	270	286	270
q15	530	488	487	487
q16	602	572	538	538
q17	1784	1464	1469	1464
q18	7901	7455	7420	7420
q19	1667	1493	1477	1477
q20	2072	1827	1839	1827
q21	5370	5024	5299	5024
q22	1118	1034	1044	1034
Total cold run time: 65583 ms
Total hot run time: 54761 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 187626 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 4563a2a7e047f711a3e431bc705a1448a40c858d, data reload: false

query1	923	387	381	381
query2	6483	1974	1944	1944
query3	6650	204	218	204
query4	33895	23161	23288	23161
query5	4248	523	483	483
query6	261	165	165	165
query7	4583	293	298	293
query8	300	220	220	220
query9	8560	2487	2495	2487
query10	450	276	274	274
query11	17404	15133	15077	15077
query12	153	97	96	96
query13	1646	415	378	378
query14	9888	7280	7270	7270
query15	272	172	170	170
query16	8113	463	440	440
query17	1599	603	539	539
query18	2122	286	290	286
query19	278	142	146	142
query20	117	111	118	111
query21	209	102	103	102
query22	4307	4182	4028	4028
query23	34147	33322	33737	33322
query24	11133	2904	2855	2855
query25	632	386	397	386
query26	1183	159	160	159
query27	2494	277	276	276
query28	7093	2065	2051	2051
query29	846	408	408	408
query30	311	157	152	152
query31	1003	804	792	792
query32	101	54	55	54
query33	759	305	295	295
query34	960	473	484	473
query35	864	719	712	712
query36	1102	952	945	945
query37	160	89	85	85
query38	4051	3855	3825	3825
query39	1434	1423	1402	1402
query40	196	113	114	113
query41	49	46	46	46
query42	120	96	94	94
query43	513	452	471	452
query44	1231	769	745	745
query45	198	165	167	165
query46	1091	748	730	730
query47	1888	1821	1824	1821
query48	405	301	295	295
query49	1094	435	447	435
query50	823	411	423	411
query51	7074	6816	6789	6789
query52	128	86	84	84
query53	249	183	181	181
query54	878	458	451	451
query55	78	76	73	73
query56	276	253	248	248
query57	1197	1062	1076	1062
query58	251	223	236	223
query59	3049	2833	2629	2629
query60	302	267	274	267
query61	110	108	100	100
query62	876	645	663	645
query63	217	187	186	186
query64	4274	746	663	663
query65	3241	3174	3169	3169
query66	1431	338	333	333
query67	15597	15207	15362	15207
query68	3570	587	567	567
query69	404	280	279	279
query70	1136	1149	1111	1111
query71	331	285	273	273
query72	6344	4056	3961	3961
query73	746	329	326	326
query74	9207	8798	8865	8798
query75	3397	2739	2672	2672
query76	1854	1041	957	957
query77	467	311	315	311
query78	9770	9070	9078	9070
query79	1031	539	524	524
query80	780	514	514	514
query81	458	236	235	235
query82	251	142	154	142
query83	173	155	157	155
query84	231	83	79	79
query85	691	283	277	277
query86	306	259	268	259
query87	4354	4197	4339	4197
query88	3160	2382	2380	2380
query89	384	289	281	281
query90	1870	192	193	192
query91	126	103	101	101
query92	60	48	52	48
query93	1037	542	538	538
query94	682	303	277	277
query95	374	270	267	267
query96	594	277	270	270
query97	3244	3073	3058	3058
query98	225	201	191	191
query99	1482	1256	1319	1256
Total cold run time: 286309 ms
Total hot run time: 187626 ms

@doris-robot
Copy link

ClickBench: Total hot run time: 33.61 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 4563a2a7e047f711a3e431bc705a1448a40c858d, data reload: false

query1	0.04	0.04	0.04
query2	0.07	0.04	0.04
query3	0.22	0.05	0.05
query4	1.67	0.08	0.08
query5	0.51	0.50	0.50
query6	1.13	0.72	0.72
query7	0.02	0.02	0.01
query8	0.05	0.04	0.05
query9	0.56	0.50	0.50
query10	0.54	0.56	0.56
query11	0.17	0.12	0.11
query12	0.15	0.12	0.12
query13	0.59	0.60	0.58
query14	2.04	2.11	2.14
query15	0.84	0.83	0.81
query16	0.38	0.38	0.38
query17	1.04	0.98	1.03
query18	0.21	0.21	0.21
query19	1.79	1.75	1.77
query20	0.02	0.01	0.01
query21	15.39	0.66	0.65
query22	4.08	5.84	3.44
query23	18.26	1.35	1.33
query24	2.11	0.24	0.22
query25	0.16	0.08	0.08
query26	0.27	0.18	0.18
query27	0.07	0.08	0.08
query28	13.21	1.00	0.99
query29	12.66	3.33	3.32
query30	0.25	0.06	0.06
query31	2.89	0.40	0.40
query32	3.26	0.49	0.48
query33	3.00	2.97	2.98
query34	17.01	4.40	4.30
query35	4.45	4.45	4.50
query36	0.67	0.47	0.49
query37	0.19	0.15	0.16
query38	0.14	0.14	0.15
query39	0.05	0.04	0.04
query40	0.16	0.12	0.14
query41	0.10	0.05	0.05
query42	0.06	0.05	0.05
query43	0.05	0.04	0.04
Total cold run time: 110.53 s
Total hot run time: 33.61 s

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.84% (9395/25505)
Line Coverage: 28.28% (77465/273945)
Region Coverage: 27.67% (39979/144460)
Branch Coverage: 24.32% (20348/83666)
Coverage Report: http://coverage.selectdb-in.cc/coverage/4563a2a7e047f711a3e431bc705a1448a40c858d_4563a2a7e047f711a3e431bc705a1448a40c858d/report/index.html

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 6, 2024

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

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Sep 6, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 6, 2024

PR approved by anyone and no changes requested.

@yiguolei yiguolei merged commit ca52591 into apache:master Sep 6, 2024
25 of 30 checks passed
zhangstar333 added a commit to zhangstar333/incubator-doris that referenced this pull request Sep 11, 2024
@zhangstar333
[Bug](compatibility) fix stddev_samp function coredump when upgrade (a…
9481d21 
…pache#40438)

## Proposed changes

```
            if (IAggregateFunction::version < AGG_FUNCTION_NULLABLE) {
                return make_nullable(Data::get_return_type());
            } else {
                return Data::get_return_type();
            }
```

before check two version , and then get return type,
but now in branch-21, the IAggregateFunction::version have update to 5,
and also AGG_FUNCTION_NEW=5
so the check will not get right return type.


<!--Describe your changes.-->
dataroaring pushed a commit that referenced this pull request Sep 11, 2024
@zhangstar333 @dataroaring
[Bug](compatibility) fix stddev_samp function coredump when upgrade (#…
bd2453e 
…40438)

## Proposed changes

```
            if (IAggregateFunction::version < AGG_FUNCTION_NULLABLE) {
                return make_nullable(Data::get_return_type());
            } else {
                return Data::get_return_type();
            }
```

before check two version , and then get return type,
but now in branch-21, the IAggregateFunction::version have update to 5,
and also AGG_FUNCTION_NEW=5
so the check will not get right return type.


<!--Describe your changes.-->
@zhangstar333 zhangstar333 mentioned this pull request Sep 11, 2024
Merged
zhangstar333 added a commit that referenced this pull request Sep 12, 2024
@zhangstar333
[cherry-pick](branch-30) fix agg functions coredump when upgrade (#40438
62d5022 
)(#40472) (#40692)

## Proposed changes

cherry-pick from master #40438
#40472

<!--Describe your changes.-->
@gavinchou gavinchou mentioned this pull request Oct 13, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BiteTheDDDDt BiteTheDDDDt BiteTheDDDDt approved these changes

@hello-stephen hello-stephen hello-stephen approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@zhangstar333 @doris-robot @BiteTheDDDDt @hello-stephen @yiguolei @dataroaring