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

[bugfix]json_length() BE crash fix #32145

Merged
merged 1 commit into from Mar 14, 2024
Merged

[bugfix]json_length() BE crash fix #32145

merged 1 commit into from Mar 14, 2024

Conversation

rohitrs1983
Copy link
Contributor

@rohitrs1983 rohitrs1983 commented Mar 12, 2024 •
edited

Proposed changes

Issue Number: close #32124

MySQL [(none)]> select json_length('abc','$.k1');
+------------------------------------------+
| json_length('abc', '$.k1') |
+------------------------------------------+
| NULL |
+------------------------------------------+
1 row in set (0.109 sec)

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...

@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

@rohitrs1983 rohitrs1983 force-pushed the json_core_fix branch 2 times, most recently from 3f802b7 to dd7969a Compare March 12, 2024 22:36
@rohitrs1983 rohitrs1983 force-pushed the json_core_fix branch 2 times, most recently from 395a6fa to 3c62ebe Compare March 13, 2024 01:46
cambyzju
cambyzju approved these changes Mar 13, 2024
View reviewed changes
Copy link
Contributor

@cambyzju cambyzju left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Mar 13, 2024
@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.

@cambyzju cambyzju added the usercase Important user case type label label Mar 13, 2024
yiguolei
yiguolei approved these changes Mar 13, 2024
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
Copy link
Contributor

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	18011	4336	4177	4177
q2	2213	159	146	146
q3	11162	939	951	939
q4	5299	703	729	703
q5	8016	2881	2898	2881
q6	188	125	128	125
q7	1188	850	817	817
q8	9650	2138	2097	2097
q9	7452	6521	6506	6506
q10	8131	2466	2496	2466
q11	421	224	226	224
q12	829	297	303	297
q13	17956	3013	2968	2968
q14	282	253	250	250
q15	497	451	450	450
q16	471	390	403	390
q17	941	533	559	533
q18	6664	5824	5776	5776
q19	1555	1491	1390	1390
q20	555	273	289	273
q21	7365	3250	3256	3250
q22	801	312	333	312
Total cold run time: 109647 ms
Total hot run time: 36970 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4141	4056	4099	4056
q2	318	222	220	220
q3	2971	2870	2820	2820
q4	1819	1534	1513	1513
q5	5211	5234	5253	5234
q6	196	114	124	114
q7	2243	1832	1870	1832
q8	3128	3249	3285	3249
q9	8601	8591	8586	8586
q10	6174	3669	3697	3669
q11	530	432	449	432
q12	722	542	592	542
q13	8995	2890	2842	2842
q14	262	246	257	246
q15	475	451	449	449
q16	449	417	410	410
q17	1706	1495	1452	1452
q18	7714	7261	7250	7250
q19	1636	1500	1561	1500
q20	1926	1719	1704	1704
q21	4853	4706	4665	4665
q22	531	476	455	455
Total cold run time: 64601 ms
Total hot run time: 53240 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.01% (8576/24499)
Line Coverage: 26.76% (69475/259623)
Region Coverage: 26.03% (36081/138609)
Branch Coverage: 22.98% (18421/80152)
Coverage Report: http://coverage.selectdb-in.cc/coverage/3c62ebe28fffd4eb4f1d8d634be9700c01c0995d_3c62ebe28fffd4eb4f1d8d634be9700c01c0995d/report/index.html

@doris-robot
Copy link

TPC-DS: Total hot run time: 186750 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 3c62ebe28fffd4eb4f1d8d634be9700c01c0995d, data reload: false

query1	923	356	336	336
query2	7501	2076	2034	2034
query3	6720	215	217	215
query4	31186	20729	20861	20729
query5	4423	417	396	396
query6	261	176	165	165
query7	4625	291	284	284
query8	228	168	158	158
query9	9888	2276	2281	2276
query10	440	241	239	239
query11	14594	14109	14156	14109
query12	133	88	84	84
query13	1631	414	430	414
query14	11708	10605	10251	10251
query15	260	212	201	201
query16	8080	263	244	244
query17	1939	554	512	512
query18	2073	286	282	282
query19	259	152	144	144
query20	88	91	79	79
query21	198	138	130	130
query22	4652	4555	4572	4555
query23	31783	30902	31137	30902
query24	10797	2826	2834	2826
query25	629	391	365	365
query26	1417	165	154	154
query27	3033	350	341	341
query28	8142	1891	1852	1852
query29	927	667	615	615
query30	346	149	151	149
query31	962	731	719	719
query32	98	61	55	55
query33	765	259	254	254
query34	1059	470	473	470
query35	832	600	588	588
query36	1016	875	859	859
query37	128	75	77	75
query38	3334	3184	3187	3184
query39	1393	1366	1375	1366
query40	202	110	110	110
query41	53	47	47	47
query42	103	92	97	92
query43	495	467	457	457
query44	1308	736	724	724
query45	271	247	256	247
query46	1110	688	682	682
query47	1669	1605	1616	1605
query48	434	351	350	350
query49	1141	342	343	342
query50	751	410	372	372
query51	6591	6580	6634	6580
query52	103	92	87	87
query53	343	275	276	275
query54	298	235	234	234
query55	83	80	82	80
query56	268	233	224	224
query57	1089	1025	1024	1024
query58	231	199	203	199
query59	2733	2589	2568	2568
query60	264	282	246	246
query61	110	108	107	107
query62	591	403	393	393
query63	296	266	274	266
query64	5632	3942	3754	3754
query65	3123	3018	2999	2999
query66	1448	334	320	320
query67	15173	14628	14485	14485
query68	7032	527	529	527
query69	643	378	374	374
query70	1272	1187	1155	1155
query71	528	290	267	267
query72	6889	2651	2471	2471
query73	728	319	313	313
query74	6847	6527	6598	6527
query75	9394	7966	7988	7966
query76	5012	927	999	927
query77	641	258	252	252
query78	10488	9683	9772	9683
query79	11315	519	511	511
query80	1757	392	368	368
query81	530	207	208	207
query82	866	203	205	203
query83	207	140	140	140
query84	281	74	75	74
query85	1504	321	302	302
query86	480	302	294	294
query87	3500	3298	3258	3258
query88	5185	2265	2246	2246
query89	524	358	361	358
query90	1949	172	170	170
query91	159	126	125	125
query92	55	45	46	45
query93	7570	513	493	493
query94	1231	177	172	172
query95	435	326	321	321
query96	603	271	264	264
query97	4436	4291	4297	4291
query98	227	220	213	213
query99	1274	782	753	753
Total cold run time: 317946 ms
Total hot run time: 186750 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.03
query2	0.06	0.02	0.02
query3	0.23	0.06	0.06
query4	1.66	0.10	0.10
query5	0.52	0.51	0.52
query6	1.20	0.63	0.62
query7	0.02	0.01	0.02
query8	0.04	0.02	0.02
query9	0.55	0.50	0.50
query10	0.55	0.56	0.54
query11	0.12	0.09	0.10
query12	0.11	0.10	0.09
query13	0.57	0.56	0.57
query14	0.75	0.76	0.75
query15	0.79	0.77	0.77
query16	0.40	0.38	0.38
query17	0.95	0.99	0.94
query18	0.20	0.25	0.24
query19	1.71	1.80	1.64
query20	0.01	0.01	0.01
query21	15.41	0.58	0.56
query22	2.64	2.25	1.35
query23	17.24	0.81	0.70
query24	3.07	0.60	1.60
query25	0.10	0.08	0.13
query26	0.63	0.13	0.12
query27	0.05	0.04	0.03
query28	11.07	0.90	0.85
query29	12.55	3.17	3.10
query30	0.66	0.61	0.57
query31	2.81	0.34	0.34
query32	3.37	0.43	0.43
query33	2.74	2.80	2.81
query34	16.12	4.34	4.31
query35	4.37	4.34	4.36
query36	1.06	1.00	1.01
query37	0.07	0.06	0.05
query38	0.04	0.03	0.03
query39	0.02	0.02	0.02
query40	0.16	0.12	0.13
query41	0.08	0.02	0.02
query42	0.03	0.01	0.01
query43	0.03	0.02	0.02
Total cold run time: 104.8 s
Total hot run time: 29.33 s

@doris-robot
Copy link

Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Load test result on commit 3c62ebe28fffd4eb4f1d8d634be9700c01c0995d with default session variables
Stream load json:         18 seconds loaded 2358488459 Bytes, about 124 MB/s
Stream load orc:          59 seconds loaded 1101869774 Bytes, about 17 MB/s
Stream load parquet:      31 seconds loaded 861443392 Bytes, about 26 MB/s
Insert into select:       13.9 seconds inserted 10000000 Rows, about 719K ops/s

@yiguolei yiguolei merged commit ee0a461 into apache:master Mar 14, 2024
24 of 29 checks passed
@cambyzju
Copy link
Contributor

@rohitrs1983 could you also fix this problem on branch-2.0 ?

yiguolei pushed a commit that referenced this pull request Mar 15, 2024
@rohitrs1983 @Doris-Extras
[bugfix]json_length() BE crash fix (#32145)
aca7328 
Co-authored-by: Rohit Satardekar <rohitrs1983@gmail.com>
xiaokang pushed a commit to xiaokang/doris that referenced this pull request Mar 19, 2024
@rohitrs1983 @xiaokang
[bugfix]json_length() BE crash fix (apache#32145)
f4b0461 
Co-authored-by: Rohit Satardekar <rohitrs1983@gmail.com>
@xiaokang xiaokang mentioned this pull request Mar 19, 2024
Merged
@xiaokang
Copy link
Contributor

@rohitrs1983 I've picked it to branch-2.0 as #32484

xiaokang added a commit that referenced this pull request Mar 19, 2024
@xiaokang @rohitrs1983
[bugfix]json_length() BE crash fix (#32145) (#32484)
8878ebd 
Co-authored-by: Rohit Satardekar <rohitrs1983@yahoo.com>
@yiguolei yiguolei mentioned this pull request Mar 24, 2024
Open
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

@cambyzju cambyzju cambyzju approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.0.7-merged reviewed usercase Important user case type label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] json_length() function causes BE crash
5 participants
@rohitrs1983 @doris-robot @yiguolei @cambyzju @xiaokang