Skip to content

[Fix] Fix generated column, static variables should not be used in ExpressionToExpr #36824

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
morrySnow merged 2 commits into from
Jun 26, 2024
Merged

[Fix] Fix generated column, static variables should not be used in ExpressionToExpr #36824

morrySnow merged 2 commits into from
Jun 26, 2024

Conversation

@feiniaofeiafei
Copy link
Contributor

@feiniaofeiafei feiniaofeiafei commented Jun 25, 2024 •
edited
Loading

introduced by #35284
Fix generated column, static variables should not be used in ExpressionToExpr. There is only one static variable slotRefMap in the ExpressionToExpr class globally. It may be used by multiple threads at the same time and assigned repeatedly, which is problematic. The same reason applies to the modification of class slotRefRewriteRule.
No regression case added because this problem does not occur every time.

@feiniaofeiafei
[Fix](nereids) fix generated column, static variables should not be u…
0b5a0ce 
…sed in ExpressionToExpr. An error will occur when shared by multiple threads.
@feiniaofeiafei
[Fix]fix generated column, static variables should not be used in Exp…
b7e5443 
…ressionToExpr. An error will occur when shared by multiple threads.
@doris-robot
Copy link

doris-robot commented Jun 25, 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.

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jun 25, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Jun 25, 2024

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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 25, 2024

PR approved by anyone and no changes requested.

@morrySnow
Copy link
Contributor

morrySnow commented Jun 25, 2024

run buildall

@doris-robot
Copy link

doris-robot commented Jun 25, 2024

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

------ Round 1 ----------------------------------
q1	17597	4390	4302	4302
q2	2007	186	186	186
q3	10458	1185	1128	1128
q4	10184	790	762	762
q5	7566	2695	2665	2665
q6	227	138	133	133
q7	956	602	621	602
q8	9609	2110	2082	2082
q9	8683	6533	6468	6468
q10	8984	3741	3720	3720
q11	446	259	234	234
q12	475	233	224	224
q13	18625	2932	2967	2932
q14	268	217	226	217
q15	511	487	480	480
q16	527	372	372	372
q17	958	678	688	678
q18	7953	7425	7329	7329
q19	7538	1438	1464	1438
q20	649	304	323	304
q21	4919	3234	3867	3234
q22	390	336	341	336
Total cold run time: 119530 ms
Total hot run time: 39826 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4410	4331	4296	4296
q2	363	269	268	268
q3	2983	2758	2769	2758
q4	1874	1599	1625	1599
q5	5216	5256	5225	5225
q6	215	126	130	126
q7	2145	1757	1758	1757
q8	3185	3335	3338	3335
q9	8331	8267	8240	8240
q10	3909	3657	3612	3612
q11	572	487	511	487
q12	789	607	558	558
q13	16279	3018	2986	2986
q14	293	270	270	270
q15	508	481	475	475
q16	480	416	422	416
q17	1760	1492	1446	1446
q18	7673	7404	7281	7281
q19	1729	1589	1523	1523
q20	2023	1798	1788	1788
q21	4901	4871	4790	4790
q22	641	542	574	542
Total cold run time: 70279 ms
Total hot run time: 53778 ms

@doris-robot
Copy link

doris-robot commented Jun 25, 2024

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

query1	922	380	382	380
query2	6435	2389	2364	2364
query3	6635	198	201	198
query4	20209	17322	17272	17272
query5	4109	473	467	467
query6	250	166	164	164
query7	4589	294	296	294
query8	321	277	269	269
query9	8548	2390	2356	2356
query10	583	292	273	273
query11	10636	10171	10055	10055
query12	127	92	83	83
query13	1630	381	378	378
query14	10079	7531	7525	7525
query15	220	182	185	182
query16	7994	290	268	268
query17	1898	532	530	530
query18	2042	267	281	267
query19	213	147	157	147
query20	93	85	80	80
query21	210	134	121	121
query22	4491	4133	4106	4106
query23	33825	32905	33094	32905
query24	11097	2768	2886	2768
query25	617	365	360	360
query26	1513	153	151	151
query27	2958	306	314	306
query28	7524	2064	2057	2057
query29	952	639	616	616
query30	284	149	150	149
query31	957	753	738	738
query32	91	55	52	52
query33	779	283	270	270
query34	981	466	469	466
query35	756	618	605	605
query36	1097	916	922	916
query37	160	70	74	70
query38	2890	2787	2790	2787
query39	837	797	778	778
query40	267	134	125	125
query41	54	51	53	51
query42	120	96	102	96
query43	570	545	516	516
query44	1209	721	736	721
query45	201	165	160	160
query46	1083	712	720	712
query47	1866	1821	1784	1784
query48	376	309	296	296
query49	1176	414	440	414
query50	772	383	400	383
query51	6871	6680	6718	6680
query52	106	93	98	93
query53	361	285	292	285
query54	963	446	436	436
query55	73	72	71	71
query56	280	261	260	260
query57	1162	1080	1038	1038
query58	259	269	254	254
query59	3363	3005	3178	3005
query60	285	280	281	280
query61	96	93	96	93
query62	654	444	435	435
query63	326	297	286	286
query64	9967	2216	1752	1752
query65	3151	3068	3100	3068
query66	1228	339	324	324
query67	15577	15104	14758	14758
query68	4668	549	542	542
query69	536	417	366	366
query70	1167	1133	1146	1133
query71	394	315	282	282
query72	7069	5450	5473	5450
query73	740	328	322	322
query74	5902	5531	5542	5531
query75	3411	2700	2643	2643
query76	2699	900	903	900
query77	510	303	309	303
query78	10404	9994	9813	9813
query79	2653	523	515	515
query80	1155	515	506	506
query81	574	218	221	218
query82	719	104	107	104
query83	240	177	172	172
query84	237	90	92	90
query85	1681	339	330	330
query86	497	325	309	309
query87	3311	3100	3125	3100
query88	4139	2359	2347	2347
query89	469	387	379	379
query90	1802	194	191	191
query91	134	112	110	110
query92	62	49	51	49
query93	1893	514	521	514
query94	1232	204	197	197
query95	418	322	329	322
query96	599	278	262	262
query97	3276	3025	3035	3025
query98	215	213	200	200
query99	1106	843	841	841
Total cold run time: 275880 ms
Total hot run time: 173215 ms

@feiniaofeiafei feiniaofeiafei changed the title [Fix]fix generated column, static variables should not be used in ExpressionToExpr. An error will occur when shared by multiple threads. [Fix]fix generated column, static variables should not be used in ExpressionToExpr Jun 25, 2024
@feiniaofeiafei feiniaofeiafei changed the title [Fix]fix generated column, static variables should not be used in ExpressionToExpr [Fix] Fix generated column, static variables should not be used in ExpressionToExpr Jun 25, 2024
@doris-robot
Copy link

doris-robot commented Jun 25, 2024

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

query1	0.04	0.04	0.03
query2	0.08	0.04	0.04
query3	0.22	0.05	0.05
query4	1.68	0.07	0.10
query5	0.50	0.49	0.51
query6	1.13	0.72	0.73
query7	0.02	0.01	0.01
query8	0.05	0.04	0.04
query9	0.53	0.50	0.50
query10	0.52	0.54	0.53
query11	0.16	0.11	0.10
query12	0.14	0.12	0.11
query13	0.58	0.58	0.58
query14	0.77	0.78	0.80
query15	0.82	0.81	0.80
query16	0.37	0.34	0.37
query17	0.98	1.02	1.05
query18	0.21	0.25	0.24
query19	1.90	1.76	1.81
query20	0.01	0.00	0.00
query21	15.46	0.66	0.66
query22	4.30	7.16	2.34
query23	18.29	1.34	1.23
query24	2.12	0.23	0.23
query25	0.15	0.09	0.08
query26	0.27	0.18	0.17
query27	0.09	0.08	0.07
query28	13.19	1.02	1.00
query29	12.61	3.31	3.29
query30	0.25	0.06	0.05
query31	2.87	0.39	0.40
query32	3.25	0.48	0.47
query33	2.86	2.90	2.92
query34	16.95	4.43	4.45
query35	4.54	4.50	4.45
query36	0.66	0.50	0.47
query37	0.18	0.15	0.16
query38	0.14	0.14	0.14
query39	0.04	0.03	0.03
query40	0.17	0.14	0.13
query41	0.08	0.05	0.04
query42	0.06	0.04	0.04
query43	0.05	0.04	0.03
Total cold run time: 109.29 s
Total hot run time: 31 s

@feiniaofeiafei
Copy link
Contributor Author

feiniaofeiafei commented Jun 26, 2024

run external

@morrySnow morrySnow merged commit 1c5926a into apache:master Jun 26, 2024
dataroaring pushed a commit that referenced this pull request Jun 28, 2024
@feiniaofeiafei @dataroaring
[fix](generated column) static variables should not be used in Expre…
940cbbe 
…ssionToExpr (#36824)

introduced by #35284
Fix generated column, static variables should not be used in
ExpressionToExpr. There is only one static variable slotRefMap in the
ExpressionToExpr class globally. It may be used by multiple threads at
the same time and assigned repeatedly, which is problematic. The same
reason applies to the modification of class slotRefRewriteRule.
No regression case added because this problem does not occur every time.

---------

Co-authored-by: feiniaofeiafei <moailing@selectdb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@starocean999 starocean999 starocean999 approved these changes

@morrySnow morrySnow morrySnow approved these changes

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. dev/3.0.0-merged not-merge/2.1 reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@feiniaofeiafei @doris-robot @morrySnow @starocean999 @dataroaring