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.

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

[fix](sleep) sleep with character const make be crash #37681

Merged
merged 2 commits into from
Jul 15, 2024

Conversation

cambyzju
Copy link
Contributor

@cambyzju cambyzju commented Jul 11, 2024

Proposed changes

Issue Number: close #37327

Come from: #31689

After fix be will not crash any more.

mysql> select cast('1.1' as int);
+--------------------+
| cast('1.1' as INT) |
+--------------------+
|               NULL |
+--------------------+
1 row in set (0.04 sec)

mysql> select sleep('1.1');
+---------------------------+
| sleep(cast('1.1' as INT)) |
+---------------------------+
|                      NULL |
+---------------------------+
1 row in set (0.02 sec)

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

@cambyzju
Copy link
Contributor Author

run buildall

Copy link
Contributor

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	18024	4858	4372	4372
q2	2203	194	188	188
q3	10514	1160	1023	1023
q4	10193	801	795	795
q5	7544	2687	2604	2604
q6	218	135	136	135
q7	963	620	597	597
q8	9212	2048	2091	2048
q9	8753	6566	6568	6566
q10	8787	3782	3790	3782
q11	448	242	261	242
q12	395	237	231	231
q13	17768	3006	2964	2964
q14	282	249	243	243
q15	518	501	482	482
q16	497	388	374	374
q17	961	648	643	643
q18	8229	7457	7443	7443
q19	7436	1519	1321	1321
q20	673	337	337	337
q21	4837	3249	3223	3223
q22	395	336	346	336
Total cold run time: 118850 ms
Total hot run time: 39949 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4376	4224	4183	4183
q2	367	281	270	270
q3	2972	2757	2760	2757
q4	1871	1634	1617	1617
q5	5261	5309	5291	5291
q6	217	129	128	128
q7	2114	1756	1741	1741
q8	3166	3315	3295	3295
q9	8377	8362	8361	8361
q10	3860	3712	3697	3697
q11	595	499	507	499
q12	782	598	614	598
q13	16657	2926	3018	2926
q14	302	262	277	262
q15	512	469	474	469
q16	474	431	421	421
q17	1761	1474	1459	1459
q18	7709	7538	7327	7327
q19	1693	1611	1587	1587
q20	1998	1797	1798	1797
q21	4916	4733	4664	4664
q22	611	562	544	544
Total cold run time: 70591 ms
Total hot run time: 53893 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 175035 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 659fa6514c676dfb5fb10055b09c6ceead7c4c40, data reload: false

query1	926	374	358	358
query2	6446	2569	2458	2458
query3	6651	210	223	210
query4	28395	17643	17362	17362
query5	4185	498	494	494
query6	309	163	153	153
query7	4577	297	288	288
query8	329	299	293	293
query9	8674	2464	2454	2454
query10	445	271	274	271
query11	11768	10214	10078	10078
query12	137	84	81	81
query13	1627	359	359	359
query14	10412	7650	7836	7650
query15	229	189	184	184
query16	7663	323	318	318
query17	1791	559	554	554
query18	1691	281	286	281
query19	208	155	153	153
query20	92	83	84	83
query21	207	127	131	127
query22	4547	4212	4182	4182
query23	33666	32984	33412	32984
query24	12015	2969	2888	2888
query25	675	389	406	389
query26	1731	155	153	153
query27	2833	280	274	274
query28	7623	2084	2072	2072
query29	1077	654	620	620
query30	290	150	151	150
query31	953	742	757	742
query32	94	57	64	57
query33	798	317	321	317
query34	956	481	504	481
query35	700	601	604	601
query36	1086	954	928	928
query37	274	82	83	82
query38	2867	2721	2749	2721
query39	869	823	796	796
query40	288	127	126	126
query41	58	56	54	54
query42	115	99	103	99
query43	600	564	562	562
query44	1236	726	731	726
query45	198	171	167	167
query46	1094	722	743	722
query47	1846	1780	1777	1777
query48	369	302	304	302
query49	1208	427	421	421
query50	789	405	402	402
query51	6877	6829	6944	6829
query52	100	96	95	95
query53	364	288	286	286
query54	1087	457	441	441
query55	79	76	76	76
query56	285	273	265	265
query57	1143	1070	1065	1065
query58	245	257	259	257
query59	3551	3220	3319	3220
query60	298	283	321	283
query61	96	99	100	99
query62	828	650	658	650
query63	321	293	289	289
query64	10536	2222	1669	1669
query65	3215	3109	3098	3098
query66	1346	371	343	343
query67	15568	15058	15101	15058
query68	5484	546	541	541
query69	671	434	377	377
query70	1218	1143	1164	1143
query71	448	280	285	280
query72	8675	5742	5444	5444
query73	797	335	331	331
query74	5869	5545	5518	5518
query75	3903	2695	2732	2695
query76	3992	949	915	915
query77	676	318	305	305
query78	9731	9107	10381	9107
query79	4215	567	527	527
query80	1551	489	486	486
query81	581	224	225	224
query82	1228	146	138	138
query83	346	179	173	173
query84	276	92	90	90
query85	1718	357	319	319
query86	488	312	302	302
query87	3264	3132	3109	3109
query88	3782	2528	2477	2477
query89	500	392	372	372
query90	1980	203	202	202
query91	141	111	105	105
query92	67	52	50	50
query93	4063	524	504	504
query94	1437	220	224	220
query95	419	335	330	330
query96	613	277	278	277
query97	3153	3033	3023	3023
query98	215	201	197	197
query99	1474	1227	1290	1227
Total cold run time: 296531 ms
Total hot run time: 175035 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.03
query2	0.07	0.04	0.05
query3	0.23	0.05	0.05
query4	1.68	0.08	0.07
query5	0.50	0.49	0.50
query6	1.16	0.72	0.71
query7	0.02	0.02	0.02
query8	0.05	0.04	0.04
query9	0.54	0.48	0.48
query10	0.55	0.55	0.55
query11	0.16	0.12	0.12
query12	0.15	0.13	0.13
query13	0.58	0.58	0.59
query14	0.76	0.77	0.77
query15	0.84	0.83	0.82
query16	0.37	0.37	0.37
query17	0.95	1.03	1.03
query18	0.23	0.21	0.22
query19	1.82	1.70	1.69
query20	0.01	0.02	0.01
query21	15.40	0.75	0.66
query22	4.36	6.91	1.95
query23	18.30	1.34	1.27
query24	2.11	0.22	0.22
query25	0.16	0.08	0.09
query26	0.30	0.20	0.21
query27	0.46	0.23	0.23
query28	13.25	1.01	1.01
query29	12.64	3.34	3.31
query30	0.24	0.06	0.06
query31	2.87	0.39	0.39
query32	3.27	0.47	0.48
query33	2.91	2.88	2.98
query34	17.19	4.34	4.42
query35	4.41	4.43	4.37
query36	0.66	0.46	0.49
query37	0.19	0.15	0.17
query38	0.16	0.15	0.16
query39	0.04	0.03	0.04
query40	0.15	0.12	0.12
query41	0.09	0.05	0.04
query42	0.05	0.05	0.05
query43	0.05	0.04	0.04
Total cold run time: 109.97 s
Total hot run time: 30.69 s

@wm1581066 wm1581066 added the usercase Important user case type label label Jul 11, 2024
zhangstar333
zhangstar333 previously approved these changes Jul 12, 2024
@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 12, 2024
Copy link
Contributor

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

Copy link
Contributor

PR approved by anyone and no changes requested.

@@ -74,19 +74,12 @@ class FunctionSleep : public IFunction {

Copy link
Contributor

@zhiqiang-hhhh zhiqiang-hhhh Jul 12, 2024

Choose a reason for hiding this comment

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

Could you please add some comment to explain why we not using default_impl for const?
"Sleep function should not be executed during open stage, this will makes fragment prepare waiting too long, so we do not use default impl."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@cambyzju
Copy link
Contributor Author

run buildall

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Jul 12, 2024
Copy link
Contributor

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17618	4345	4346	4345
q2	2023	194	184	184
q3	10448	1209	1160	1160
q4	10201	871	773	773
q5	7554	2676	2668	2668
q6	220	139	140	139
q7	959	604	602	602
q8	9232	2093	2060	2060
q9	8750	6571	6595	6571
q10	8899	3757	3780	3757
q11	464	245	248	245
q12	403	227	230	227
q13	17758	2998	3025	2998
q14	275	239	225	225
q15	533	491	480	480
q16	484	394	382	382
q17	965	687	609	609
q18	8062	7400	7366	7366
q19	7312	1454	1515	1454
q20	662	320	327	320
q21	4880	3200	4047	3200
q22	383	334	341	334
Total cold run time: 118085 ms
Total hot run time: 40099 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4411	4262	4281	4262
q2	389	267	273	267
q3	3073	2899	2923	2899
q4	1973	1696	1763	1696
q5	5537	5591	5466	5466
q6	244	135	136	135
q7	2209	1854	1862	1854
q8	3256	3395	3447	3395
q9	8801	8811	8933	8811
q10	4108	3863	3780	3780
q11	605	505	496	496
q12	831	652	645	645
q13	16175	3170	3193	3170
q14	321	295	288	288
q15	523	490	487	487
q16	504	444	428	428
q17	1830	1516	1526	1516
q18	8146	7907	7750	7750
q19	1792	1644	1682	1644
q20	2122	1880	1911	1880
q21	5049	4840	4787	4787
q22	612	522	578	522
Total cold run time: 72511 ms
Total hot run time: 56178 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 175411 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 62dc29da0f0bb17ccaeee8c25ecda2e77b0e6e92, data reload: false

query1	910	376	372	372
query2	6442	2452	2391	2391
query3	6636	210	230	210
query4	28451	17406	17477	17406
query5	3680	499	497	497
query6	270	181	154	154
query7	4591	289	284	284
query8	310	272	285	272
query9	8586	2470	2443	2443
query10	435	280	263	263
query11	11304	10089	10095	10089
query12	115	84	80	80
query13	1638	363	368	363
query14	9617	7018	7997	7018
query15	241	180	187	180
query16	7160	323	320	320
query17	1659	543	537	537
query18	1803	273	274	273
query19	190	149	148	148
query20	99	82	84	82
query21	211	127	123	123
query22	4297	4112	3962	3962
query23	33949	33893	33526	33526
query24	11031	2948	2905	2905
query25	581	414	428	414
query26	710	159	152	152
query27	2239	278	278	278
query28	6096	2187	2178	2178
query29	886	671	627	627
query30	262	168	151	151
query31	993	752	742	742
query32	99	54	60	54
query33	706	315	316	315
query34	907	506	506	506
query35	730	609	606	606
query36	1132	995	985	985
query37	147	93	90	90
query38	2948	2915	2786	2786
query39	930	843	848	843
query40	208	126	124	124
query41	54	52	54	52
query42	116	100	99	99
query43	600	534	548	534
query44	1207	730	730	730
query45	202	172	168	168
query46	1110	721	751	721
query47	1911	1776	1785	1776
query48	363	313	308	308
query49	850	441	427	427
query50	787	399	400	399
query51	6793	6909	6857	6857
query52	110	96	102	96
query53	358	286	297	286
query54	921	466	468	466
query55	79	76	74	74
query56	302	290	293	290
query57	1116	1078	1090	1078
query58	248	279	252	252
query59	3263	3204	3172	3172
query60	324	301	292	292
query61	123	116	117	116
query62	793	666	644	644
query63	318	307	293	293
query64	9251	2309	7537	2309
query65	3189	3113	3113	3113
query66	752	338	351	338
query67	15574	15012	15023	15012
query68	6854	557	568	557
query69	721	477	368	368
query70	1162	1120	1076	1076
query71	500	288	285	285
query72	8319	5900	5442	5442
query73	767	330	330	330
query74	5924	5513	5536	5513
query75	4462	2677	2675	2675
query76	4248	993	889	889
query77	786	304	311	304
query78	10044	10095	8979	8979
query79	8119	530	538	530
query80	2705	484	485	484
query81	577	225	218	218
query82	670	135	132	132
query83	287	163	204	163
query84	265	86	89	86
query85	1287	326	303	303
query86	439	319	302	302
query87	3323	3160	3116	3116
query88	5109	2503	2553	2503
query89	509	402	391	391
query90	1940	200	195	195
query91	131	107	107	107
query92	66	51	50	50
query93	5850	511	512	511
query94	1082	215	213	213
query95	421	349	315	315
query96	620	279	271	271
query97	3236	2999	3057	2999
query98	231	202	194	194
query99	1670	1259	1261	1259
Total cold run time: 296237 ms
Total hot run time: 175411 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.04
query2	0.08	0.04	0.04
query3	0.22	0.05	0.06
query4	1.66	0.09	0.09
query5	0.49	0.47	0.50
query6	1.16	0.72	0.73
query7	0.02	0.01	0.02
query8	0.05	0.04	0.04
query9	0.55	0.50	0.49
query10	0.56	0.55	0.55
query11	0.16	0.11	0.11
query12	0.15	0.12	0.12
query13	0.58	0.59	0.59
query14	0.77	0.76	0.78
query15	0.85	0.81	0.82
query16	0.36	0.36	0.37
query17	0.96	1.06	1.04
query18	0.22	0.21	0.22
query19	1.81	1.72	1.72
query20	0.02	0.01	0.01
query21	15.40	0.75	0.66
query22	4.32	7.20	2.37
query23	18.33	1.44	1.33
query24	2.07	0.25	0.22
query25	0.17	0.08	0.09
query26	0.30	0.20	0.20
query27	0.45	0.23	0.23
query28	13.32	1.02	1.00
query29	12.70	3.31	3.24
query30	0.25	0.06	0.06
query31	2.86	0.40	0.39
query32	3.30	0.49	0.47
query33	2.91	2.90	2.91
query34	17.19	4.38	4.38
query35	4.44	4.41	4.42
query36	0.65	0.46	0.49
query37	0.19	0.16	0.15
query38	0.15	0.15	0.15
query39	0.05	0.03	0.04
query40	0.16	0.12	0.13
query41	0.10	0.05	0.04
query42	0.06	0.05	0.05
query43	0.05	0.03	0.04
Total cold run time: 110.13 s
Total hot run time: 31.19 s

bool use_default_implementation_for_constants() const override { return false; }

Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments,
size_t result, size_t input_rows_count) const override {
ColumnPtr& argument_column = block.get_by_position(arguments[0]).column;
const auto& argument_column =
Copy link
Contributor

Choose a reason for hiding this comment

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

use unpack_if_const may be better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are already lots of convert_to_full_column_if_const used, maybe next time, we refractor all these used is better.

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

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

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

Copy link
Contributor

@lide-reed lide-reed left a comment

Choose a reason for hiding this comment

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

LGTM

@lide-reed lide-reed merged commit ceead84 into apache:master Jul 15, 2024
31 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.5-merged dev/3.0.1-merged reviewed usercase Important user case type label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] sleep function with character constant(such as 'a', '1.1') leads to be crash
9 participants