Skip to content

[fix](json) fix parsing fails when json key string is empty. #39937

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
Merged
HappenLee merged 2 commits into from
Aug 27, 2024
Merged

[fix](json) fix parsing fails when json key string is empty. #39937

HappenLee merged 2 commits into from
Aug 27, 2024

Conversation

@Mryange
Copy link
Contributor

@Mryange Mryange commented Aug 26, 2024

Proposed changes

This is a very subtle bug that can only be reproduced in a non-AVX2 environment.

// USE_AVX2 = false
mysql [(none)]>mysql [(none)]>select cast('{"":1, " ":"v1"}' as json);
+----------------------------------+
| cast('{"":1, " ":"v1"}' as JSON) |
+----------------------------------+
| NULL                             |
+----------------------------------+

//USE_AVX2 = true
mysql [(none)]>select cast('{"":1, " ":"v1"}' as json);
+----------------------------------+
| cast('{"":1, " ":"v1"}' as JSON) |
+----------------------------------+
| {"":1," ":"v1"}                  |
+----------------------------------+

@doris-robot
Copy link

doris-robot commented Aug 26, 2024

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.

@Mryange
Copy link
Contributor Author

Mryange commented Aug 26, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2024

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

zclllyybb
zclllyybb reviewed Aug 26, 2024
View reviewed changes
Copy link
Contributor

@zclllyybb zclllyybb left a comment

Choose a reason for hiding this comment

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

add comment

@Mryange
Copy link
Contributor Author

Mryange commented Aug 26, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2024

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

@doris-robot
Copy link

doris-robot commented Aug 26, 2024

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

------ Round 1 ----------------------------------
q1	16129	4499	4336	4336
q2	1362	184	177	177
q3	10144	1205	1121	1121
q4	7318	786	654	654
q5	2868	2820	2803	2803
q6	230	143	142	142
q7	968	610	609	609
q8	3027	2056	2083	2056
q9	6767	6583	6546	6546
q10	4454	2179	2185	2179
q11	452	240	246	240
q12	394	222	219	219
q13	18616	3058	3048	3048
q14	282	233	252	233
q15	524	499	496	496
q16	502	399	386	386
q17	987	680	718	680
q18	7611	7005	6831	6831
q19	1237	1040	988	988
q20	677	344	325	325
q21	3839	2817	2900	2817
q22	1142	1009	1005	1005
Total cold run time: 89530 ms
Total hot run time: 37891 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4345	4304	4283	4283
q2	372	273	269	269
q3	2895	2694	2600	2600
q4	1905	1649	1618	1618
q5	5440	5396	5407	5396
q6	223	128	131	128
q7	2120	1730	1707	1707
q8	3179	3360	3335	3335
q9	8494	8464	8467	8464
q10	3453	3248	3214	3214
q11	588	493	506	493
q12	789	630	593	593
q13	12080	3092	3054	3054
q14	311	287	270	270
q15	511	479	468	468
q16	480	421	441	421
q17	1795	1472	1476	1472
q18	7777	7391	7485	7391
q19	1670	1607	1566	1566
q20	2025	1813	1802	1802
q21	5390	5187	5189	5187
q22	1147	986	1053	986
Total cold run time: 66989 ms
Total hot run time: 54717 ms

@doris-robot
Copy link

doris-robot commented Aug 26, 2024

TPC-DS: Total hot run time: 186589 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 7a789bbd692adb5f639f09be3f8cd37eee747b37, data reload: false

query1	921	385	360	360
query2	6472	1991	1955	1955
query3	6650	211	223	211
query4	33785	23127	23102	23102
query5	4181	504	488	488
query6	264	196	185	185
query7	4587	287	289	287
query8	255	208	211	208
query9	8466	2447	2416	2416
query10	434	263	265	263
query11	17731	15048	15056	15048
query12	142	106	100	100
query13	1633	371	382	371
query14	9748	7305	7119	7119
query15	290	170	179	170
query16	8082	481	412	412
query17	1558	554	566	554
query18	2145	300	317	300
query19	266	143	141	141
query20	122	114	109	109
query21	208	107	101	101
query22	4457	4238	4091	4091
query23	34006	33074	33567	33074
query24	11048	2833	2833	2833
query25	625	376	395	376
query26	1156	158	156	156
query27	2611	278	273	273
query28	7534	2051	2023	2023
query29	813	408	395	395
query30	298	155	146	146
query31	991	777	779	777
query32	105	57	63	57
query33	747	290	290	290
query34	969	469	484	469
query35	883	735	754	735
query36	1111	941	939	939
query37	154	91	86	86
query38	4009	3903	3900	3900
query39	1453	1378	1384	1378
query40	203	115	117	115
query41	49	46	45	45
query42	113	97	97	97
query43	524	485	466	466
query44	1215	732	741	732
query45	196	171	166	166
query46	1102	739	733	733
query47	1910	1785	1776	1776
query48	385	282	285	282
query49	1088	433	424	424
query50	824	410	406	406
query51	7298	7069	7067	7067
query52	100	88	91	88
query53	252	178	177	177
query54	891	473	457	457
query55	74	74	78	74
query56	279	260	256	256
query57	1212	1073	1078	1073
query58	247	216	228	216
query59	2940	2834	2750	2750
query60	305	265	276	265
query61	101	97	102	97
query62	903	661	672	661
query63	219	183	184	183
query64	5365	2325	1753	1753
query65	3240	3156	3160	3156
query66	1409	353	333	333
query67	15456	15238	15583	15238
query68	3136	576	572	572
query69	400	288	275	275
query70	1190	1121	1145	1121
query71	340	271	271	271
query72	2767	2083	2077	2077
query73	697	319	318	318
query74	9137	8850	8816	8816
query75	3392	2620	2724	2620
query76	1868	950	992	950
query77	483	318	320	318
query78	9591	9007	9050	9007
query79	1030	535	528	528
query80	722	519	531	519
query81	452	232	228	228
query82	290	141	139	139
query83	186	160	161	160
query84	228	84	81	81
query85	773	355	275	275
query86	304	294	291	291
query87	4285	4311	4248	4248
query88	3055	2299	2287	2287
query89	388	290	290	290
query90	1930	201	198	198
query91	121	101	98	98
query92	63	54	54	54
query93	1027	529	536	529
query94	681	303	279	279
query95	340	256	255	255
query96	587	273	269	269
query97	3200	3072	3077	3072
query98	265	209	197	197
query99	1476	1271	1263	1263
Total cold run time: 283214 ms
Total hot run time: 186589 ms

@doris-robot
Copy link

doris-robot commented Aug 26, 2024

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

query1	0.04	0.04	0.04
query2	0.09	0.04	0.04
query3	0.22	0.05	0.05
query4	1.67	0.08	0.08
query5	0.50	0.50	0.50
query6	1.13	0.73	0.73
query7	0.02	0.01	0.01
query8	0.05	0.05	0.05
query9	0.55	0.49	0.47
query10	0.52	0.54	0.53
query11	0.16	0.11	0.12
query12	0.16	0.13	0.12
query13	0.61	0.58	0.59
query14	0.78	0.76	0.79
query15	0.89	0.81	0.81
query16	0.35	0.38	0.37
query17	0.99	1.01	1.00
query18	0.21	0.20	0.20
query19	1.87	1.68	1.80
query20	0.01	0.01	0.02
query21	15.39	0.67	0.66
query22	4.43	7.94	1.58
query23	18.31	1.37	1.25
query24	2.08	0.24	0.22
query25	0.15	0.08	0.08
query26	0.28	0.18	0.17
query27	0.08	0.08	0.07
query28	13.25	1.01	1.00
query29	12.68	3.40	3.37
query30	0.24	0.06	0.06
query31	2.88	0.38	0.39
query32	3.25	0.47	0.48
query33	2.96	3.01	3.02
query34	17.14	4.40	4.45
query35	4.48	4.48	4.48
query36	0.66	0.47	0.51
query37	0.20	0.17	0.15
query38	0.16	0.15	0.15
query39	0.05	0.04	0.04
query40	0.15	0.13	0.14
query41	0.09	0.04	0.05
query42	0.06	0.05	0.05
query43	0.05	0.05	0.04
Total cold run time: 109.84 s
Total hot run time: 30.41 s

zclllyybb
zclllyybb approved these changes Aug 26, 2024
View reviewed changes
Copy link
Contributor

@zclllyybb zclllyybb 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
Copy link
Contributor

github-actions bot commented Aug 26, 2024

PR approved by anyone and no changes requested.

HappenLee
HappenLee approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@HappenLee HappenLee 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 Aug 27, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Aug 27, 2024

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

@HappenLee HappenLee merged commit 62990d8 into apache:master Aug 27, 2024
dataroaring pushed a commit that referenced this pull request Oct 9, 2024
@Mryange @dataroaring
[fix](json) fix parsing fails when json key string is empty. (#39937)
2e6903b 
This is a very subtle bug that can only be reproduced in a non-AVX2
environment.
```
// USE_AVX2 = false
mysql [(none)]>mysql [(none)]>select cast('{"":1, " ":"v1"}' as json);
+----------------------------------+
| cast('{"":1, " ":"v1"}' as JSON) |
+----------------------------------+
| NULL                             |
+----------------------------------+

//USE_AVX2 = true
mysql [(none)]>select cast('{"":1, " ":"v1"}' as json);
+----------------------------------+
| cast('{"":1, " ":"v1"}' as JSON) |
+----------------------------------+
| {"":1," ":"v1"}                  |
+----------------------------------+
```
@gavinchou gavinchou mentioned this pull request Nov 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HappenLee HappenLee HappenLee approved these changes

@zclllyybb zclllyybb zclllyybb approved these changes

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. dev/3.0.3-merged reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Mryange @doris-robot @HappenLee @zclllyybb @dataroaring