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

[fix](expr) Enhance SQL Expression Handling by Introducing printSqlInParens to CompoundPredicate #39064

Merged
merged 2 commits into from
Aug 13, 2024
Merged

[fix](expr) Enhance SQL Expression Handling by Introducing printSqlInParens to CompoundPredicate #39064

merged 2 commits into from
Aug 13, 2024

Conversation

zy-kkk
Copy link
Member

@zy-kkk zy-kkk commented Aug 7, 2024 •
edited
Loading

This PR enhances SQL expression handling by introducing a printSqlInParens flag to the CompoundPredicate class and setting it to true. This ensures that expressions in a CompoundPredicate are always enclosed in parentheses, similar to how it is handled in a BinaryPredicate. . By removing redundant handling of CompoundPredicate, the logic for handling SQL generation in the JdbcScanNode node has been simplified as the updated toSql method now handles this uniformly across expressions.

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

@zy-kkk
Copy link
Member Author

zy-kkk commented Aug 7, 2024

run buildall

@github-actions github-actions bot added the doing label Aug 7, 2024
@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17820	4440	4311	4311
q2	2022	173	176	173
q3	10535	1209	1086	1086
q4	10159	767	665	665
q5	7504	2497	2478	2478
q6	228	141	143	141
q7	954	589	593	589
q8	9220	1901	1925	1901
q9	8711	6527	6525	6525
q10	7061	2164	2161	2161
q11	484	244	242	242
q12	390	228	221	221
q13	18237	2980	2941	2941
q14	279	244	235	235
q15	523	492	498	492
q16	522	394	385	385
q17	963	659	727	659
q18	8199	7443	7397	7397
q19	5941	1054	1015	1015
q20	707	331	328	328
q21	5401	4281	4186	4186
q22	1088	1021	977	977
Total cold run time: 116948 ms
Total hot run time: 39108 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4418	4287	4238	4238
q2	381	268	267	267
q3	2846	2634	2606	2606
q4	1892	1643	1616	1616
q5	5472	5504	5491	5491
q6	243	135	134	134
q7	2104	1703	1750	1703
q8	3272	3414	3415	3414
q9	8703	8908	8708	8708
q10	3489	3271	3203	3203
q11	570	485	499	485
q12	794	639	660	639
q13	16526	3144	3148	3144
q14	325	291	292	291
q15	546	489	501	489
q16	486	452	432	432
q17	1850	1520	1485	1485
q18	8195	7509	7383	7383
q19	4245	1623	1389	1389
q20	2706	1794	1788	1788
q21	5298	5055	5100	5055
q22	1145	1019	968	968
Total cold run time: 75506 ms
Total hot run time: 54928 ms

@zy-kkk
Copy link
Member Author

zy-kkk commented Aug 8, 2024

run buildall

@zy-kkk zy-kkk mentioned this pull request Aug 8, 2024
Merged
@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17629	4353	4231	4231
q2	2014	182	168	168
q3	10503	1135	1120	1120
q4	10147	833	770	770
q5	7507	2521	2437	2437
q6	223	137	136	136
q7	960	607	585	585
q8	9216	1913	2017	1913
q9	8688	6563	6535	6535
q10	7113	2149	2202	2149
q11	462	237	244	237
q12	451	218	215	215
q13	18862	2958	3005	2958
q14	287	236	235	235
q15	526	485	483	483
q16	510	392	381	381
q17	969	704	662	662
q18	8028	7556	7425	7425
q19	4286	1055	969	969
q20	665	334	335	334
q21	5280	4660	4459	4459
q22	1110	994	1000	994
Total cold run time: 115436 ms
Total hot run time: 39396 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4406	4238	4243	4238
q2	378	277	264	264
q3	2845	2624	2664	2624
q4	1954	1681	1704	1681
q5	5545	5542	5501	5501
q6	224	130	127	127
q7	2095	1727	1771	1727
q8	3262	3457	3434	3434
q9	8770	8757	8854	8757
q10	3472	3202	3274	3202
q11	585	510	494	494
q12	767	612	614	612
q13	17091	3178	3154	3154
q14	312	279	291	279
q15	542	482	471	471
q16	501	432	439	432
q17	1830	1505	1524	1505
q18	8032	8014	7752	7752
q19	1711	1617	1700	1617
q20	2105	1898	1860	1860
q21	5329	5422	5158	5158
q22	1079	993	998	993
Total cold run time: 72835 ms
Total hot run time: 55882 ms

@zy-kkk zy-kkk changed the title [fix](expr) Add Parentheses for Correct Operator Precedence in toSqlImpl and conjunctExprToString Methods [fix](expr) Enhance SQL Expression Handling by Introducing printSqlInParens to CompoundPredicate Aug 12, 2024
@zy-kkk zy-kkk marked this pull request as ready for review August 12, 2024 11:35
@zy-kkk
Copy link
Member Author

zy-kkk commented Aug 12, 2024

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17647	4842	4276	4276
q2	2022	198	200	198
q3	11878	1025	1115	1025
q4	10504	675	714	675
q5	7495	2492	2552	2492
q6	218	137	134	134
q7	958	600	583	583
q8	9437	1894	1913	1894
q9	8642	6518	6582	6518
q10	7046	2209	2205	2205
q11	442	246	252	246
q12	387	223	215	215
q13	17905	2955	2959	2955
q14	284	240	230	230
q15	522	490	475	475
q16	493	398	384	384
q17	949	778	710	710
q18	8120	7438	7242	7242
q19	4390	974	1017	974
q20	680	338	332	332
q21	5484	4392	4169	4169
q22	1106	996	995	995
Total cold run time: 116609 ms
Total hot run time: 38927 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4440	4267	4283	4267
q2	361	284	284	284
q3	2846	2627	2775	2627
q4	2029	1755	1678	1678
q5	5653	5472	5369	5369
q6	221	132	128	128
q7	2117	1841	1779	1779
q8	3272	3439	3405	3405
q9	8731	8862	8770	8770
q10	3505	3221	3151	3151
q11	588	517	520	517
q12	822	677	611	611
q13	17304	3132	3182	3132
q14	317	283	299	283
q15	527	486	486	486
q16	501	423	440	423
q17	1826	1516	1505	1505
q18	8079	7852	7841	7841
q19	1750	1589	1550	1550
q20	2138	1855	1845	1845
q21	5601	5485	5087	5087
q22	1140	1047	1028	1028
Total cold run time: 73768 ms
Total hot run time: 55766 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 202367 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 5423adda60d74a2e1a4ddd810af2ab7a9878da46, data reload: false

query1	1589	1465	1433	1433
query2	6445	1975	1876	1876
query3	13834	8186	277	277
query4	33402	23224	23132	23132
query5	4208	501	503	501
query6	271	176	169	169
query7	3940	309	297	297
query8	252	205	209	205
query9	8562	2484	2468	2468
query10	560	449	423	423
query11	15498	14940	15075	14940
query12	144	96	97	96
query13	1532	375	355	355
query14	9318	7146	7693	7146
query15	292	224	215	215
query16	7660	518	460	460
query17	1668	610	587	587
query18	1951	291	285	285
query19	190	148	147	147
query20	132	105	109	105
query21	202	109	103	103
query22	4571	4316	4237	4237
query23	35986	33618	33709	33618
query24	8527	2531	2542	2531
query25	492	374	372	372
query26	682	151	146	146
query27	2039	279	281	279
query28	5871	2046	2042	2042
query29	623	411	416	411
query30	247	149	148	148
query31	944	781	740	740
query32	93	53	56	53
query33	595	288	275	275
query34	846	472	470	470
query35	934	821	812	812
query36	1072	944	938	938
query37	139	77	83	77
query38	4277	4160	4129	4129
query39	1456	1356	1366	1356
query40	214	122	114	114
query41	45	44	44	44
query42	118	101	95	95
query43	508	481	477	477
query44	1084	723	725	723
query45	231	205	239	205
query46	1094	733	749	733
query47	1831	1731	1748	1731
query48	380	295	301	295
query49	825	415	413	413
query50	815	416	408	408
query51	6880	6686	6672	6672
query52	102	92	89	89
query53	257	180	176	176
query54	781	453	455	453
query55	78	74	72	72
query56	268	245	254	245
query57	1147	1082	1091	1082
query58	241	235	234	234
query59	3065	2751	2671	2671
query60	290	259	264	259
query61	103	93	96	93
query62	784	625	641	625
query63	214	185	178	178
query64	9163	2279	1738	1738
query65	3261	3136	3158	3136
query66	670	327	335	327
query67	15283	14783	14749	14749
query68	4532	549	553	549
query69	426	402	404	402
query70	1141	1137	1091	1091
query71	387	281	275	275
query72	17929	16774	17289	16774
query73	762	383	334	334
query74	9239	8686	8774	8686
query75	3309	2701	2702	2701
query76	2030	1058	913	913
query77	450	305	324	305
query78	9506	9000	8872	8872
query79	2079	522	521	521
query80	2014	497	481	481
query81	611	226	223	223
query82	752	140	141	140
query83	286	149	143	143
query84	265	83	84	83
query85	1883	300	275	275
query86	468	262	293	262
query87	4650	4485	4675	4485
query88	3978	2508	2487	2487
query89	409	284	288	284
query90	1650	206	203	203
query91	138	107	114	107
query92	63	52	53	52
query93	3213	542	537	537
query94	759	306	380	306
query95	361	266	264	264
query96	618	277	278	277
query97	3236	3053	3016	3016
query98	229	205	200	200
query99	1669	1259	1228	1228
Total cold run time: 309818 ms
Total hot run time: 202367 ms

@doris-robot
Copy link

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

query1	0.04	0.05	0.04
query2	0.10	0.04	0.04
query3	0.23	0.05	0.05
query4	1.66	0.09	0.08
query5	0.49	0.48	0.50
query6	1.13	0.74	0.73
query7	0.02	0.01	0.01
query8	0.05	0.04	0.04
query9	0.55	0.49	0.49
query10	0.55	0.54	0.54
query11	0.15	0.11	0.11
query12	0.14	0.13	0.12
query13	0.59	0.60	0.58
query14	0.77	0.79	0.78
query15	0.86	0.84	0.81
query16	0.35	0.36	0.37
query17	0.99	0.99	0.96
query18	0.23	0.22	0.21
query19	1.79	1.71	1.74
query20	0.01	0.01	0.00
query21	15.39	0.77	0.64
query22	4.59	6.33	2.54
query23	18.31	1.35	1.32
query24	2.13	0.22	0.22
query25	0.15	0.08	0.08
query26	0.32	0.21	0.22
query27	0.76	0.23	0.23
query28	13.92	1.02	1.02
query29	12.62	3.32	3.25
query30	0.24	0.05	0.05
query31	2.87	0.39	0.39
query32	3.28	0.48	0.47
query33	2.84	2.92	2.86
query34	17.13	4.30	4.38
query35	4.44	4.38	4.41
query36	0.65	0.46	0.47
query37	0.19	0.16	0.15
query38	0.15	0.16	0.15
query39	0.04	0.04	0.04
query40	0.14	0.12	0.13
query41	0.10	0.05	0.05
query42	0.06	0.04	0.05
query43	0.04	0.04	0.04
Total cold run time: 111.06 s
Total hot run time: 31.15 s

@zy-kkk
Copy link
Member Author

zy-kkk commented Aug 12, 2024

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	18182	4731	4427	4427
q2	2500	181	192	181
q3	11155	1156	1079	1079
q4	11234	840	697	697
q5	7924	2614	2611	2611
q6	229	141	141	141
q7	1001	604	614	604
q8	9441	1958	2037	1958
q9	8834	6583	6571	6571
q10	7020	2211	2220	2211
q11	469	247	247	247
q12	395	213	220	213
q13	17788	2982	2976	2976
q14	282	233	224	224
q15	520	477	480	477
q16	500	405	392	392
q17	987	648	704	648
q18	8232	7545	7582	7545
q19	6636	1070	1108	1070
q20	696	324	318	318
q21	5391	4586	4530	4530
q22	1095	1021	994	994
Total cold run time: 120511 ms
Total hot run time: 40114 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4542	4282	4303	4282
q2	380	265	256	256
q3	2833	2628	2614	2614
q4	1901	1567	1605	1567
q5	5272	5255	5257	5255
q6	216	130	134	130
q7	2096	1681	1647	1647
q8	3161	3367	3321	3321
q9	8438	8363	8320	8320
q10	3351	3134	3162	3134
q11	597	501	502	501
q12	779	622	591	591
q13	17549	2967	2968	2967
q14	306	269	265	265
q15	517	492	471	471
q16	500	422	410	410
q17	1805	1493	1507	1493
q18	7828	7452	7350	7350
q19	1671	1659	1655	1655
q20	2021	1789	1794	1789
q21	5146	5063	5208	5063
q22	1081	978	990	978
Total cold run time: 71990 ms
Total hot run time: 54059 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 199718 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 4f0997fd6c1fc99dd7bfdaa520d72051fc3ee990, data reload: false

query1	911	366	359	359
query2	6456	1951	1859	1859
query3	6643	209	213	209
query4	34218	23153	23211	23153
query5	4201	501	499	499
query6	262	170	155	155
query7	4575	287	284	284
query8	238	193	187	187
query9	8770	2524	2488	2488
query10	552	451	454	451
query11	17670	14953	15150	14953
query12	147	102	100	100
query13	1637	378	366	366
query14	9470	7670	6861	6861
query15	275	220	228	220
query16	7824	491	500	491
query17	1760	587	552	552
query18	2006	285	277	277
query19	198	144	142	142
query20	111	107	105	105
query21	200	100	104	100
query22	4474	3910	4012	3910
query23	33812	33080	33103	33080
query24	12103	2681	2584	2584
query25	661	379	389	379
query26	1802	158	157	157
query27	2931	274	280	274
query28	7671	2036	2059	2036
query29	1133	427	422	422
query30	301	152	149	149
query31	968	740	696	696
query32	100	54	54	54
query33	750	294	280	280
query34	954	458	469	458
query35	945	833	816	816
query36	1050	948	935	935
query37	292	81	79	79
query38	4236	4118	4116	4116
query39	1436	1397	1379	1379
query40	272	120	113	113
query41	50	53	47	47
query42	113	97	95	95
query43	493	462	459	459
query44	1219	734	717	717
query45	225	209	204	204
query46	1119	718	751	718
query47	1877	1809	1754	1754
query48	373	296	301	296
query49	1212	420	424	420
query50	804	412	410	410
query51	6858	6758	6705	6705
query52	106	88	93	88
query53	260	184	182	182
query54	1038	452	445	445
query55	78	77	76	76
query56	269	263	245	245
query57	1180	1065	1032	1032
query58	246	229	236	229
query59	3103	2613	2770	2613
query60	306	270	262	262
query61	118	112	118	112
query62	834	666	657	657
query63	227	303	175	175
query64	10515	2227	1747	1747
query65	3211	3113	3148	3113
query66	1371	322	332	322
query67	15446	14696	14767	14696
query68	6598	543	545	543
query69	482	408	400	400
query70	1209	1026	1039	1026
query71	540	265	264	264
query72	19887	16515	16281	16281
query73	795	326	321	321
query74	9274	8627	8753	8627
query75	5208	2598	2621	2598
query76	4773	972	1022	972
query77	753	306	307	306
query78	11511	12182	9316	9316
query79	10086	515	513	513
query80	1712	485	483	483
query81	613	223	217	217
query82	293	132	132	132
query83	340	145	141	141
query84	277	75	79	75
query85	711	312	262	262
query86	435	299	310	299
query87	4761	4454	4460	4454
query88	5585	2497	2521	2497
query89	393	281	291	281
query90	2282	194	194	194
query91	120	94	93	93
query92	65	49	52	49
query93	1104	531	529	529
query94	1139	298	290	290
query95	355	258	259	258
query96	593	278	284	278
query97	3263	3029	3103	3029
query98	221	200	196	196
query99	1507	1282	1247	1247
Total cold run time: 333551 ms
Total hot run time: 199718 ms

@doris-robot
Copy link

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

query1	0.05	0.04	0.04
query2	0.09	0.04	0.04
query3	0.23	0.06	0.05
query4	1.67	0.07	0.07
query5	0.50	0.48	0.47
query6	1.15	0.71	0.71
query7	0.02	0.01	0.02
query8	0.05	0.04	0.05
query9	0.56	0.49	0.47
query10	0.53	0.53	0.53
query11	0.15	0.12	0.11
query12	0.15	0.12	0.12
query13	0.59	0.59	0.59
query14	0.76	0.79	0.78
query15	0.86	0.81	0.83
query16	0.36	0.38	0.37
query17	0.95	1.02	0.97
query18	0.22	0.22	0.21
query19	1.86	1.70	1.79
query20	0.01	0.01	0.02
query21	15.39	0.76	0.67
query22	4.52	7.63	1.53
query23	18.30	1.45	1.22
query24	2.07	0.22	0.21
query25	0.16	0.08	0.08
query26	0.29	0.21	0.21
query27	0.47	0.22	0.22
query28	13.42	1.00	1.00
query29	12.56	3.30	3.36
query30	0.23	0.05	0.04
query31	2.90	0.39	0.38
query32	3.31	0.48	0.47
query33	2.82	2.94	2.85
query34	16.98	4.36	4.35
query35	4.41	4.44	4.38
query36	0.65	0.46	0.48
query37	0.18	0.16	0.16
query38	0.16	0.16	0.15
query39	0.05	0.03	0.04
query40	0.15	0.12	0.12
query41	0.09	0.05	0.05
query42	0.06	0.05	0.05
query43	0.04	0.04	0.05
Total cold run time: 109.97 s
Total hot run time: 30.07 s

@zy-kkk
Copy link
Member Author

zy-kkk commented Aug 13, 2024

run cloud_p0

morningman
morningman approved these changes Aug 13, 2024
View reviewed changes
Copy link
Contributor

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

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 13, 2024
@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@zy-kkk zy-kkk merged commit b24cbe2 into apache:master Aug 13, 2024
30 of 31 checks passed
@zy-kkk zy-kkk deleted the fix_0807 branch August 13, 2024 09:14
zy-kkk added a commit to zy-kkk/doris that referenced this pull request Aug 13, 2024
@zy-kkk
[fix](expr) Enhance SQL Expression Handling by Introducing printSqlIn…
b23347e 
…Parens to CompoundPredicate (apache#39064)

This PR enhances SQL expression handling by introducing a printSqlInParens flag to the CompoundPredicate class and setting it to true. This ensures that expressions in a CompoundPredicate are always enclosed in parentheses, similar to how it is handled in a BinaryPredicate. . By removing redundant handling of CompoundPredicate, the logic for handling SQL generation in the JdbcScanNode node has been simplified as the updated toSql method now handles this uniformly across expressions.
wyxxxcat pushed a commit to wyxxxcat/doris that referenced this pull request Aug 14, 2024
@zy-kkk @wyxxxcat
[fix](expr) Enhance SQL Expression Handling by Introducing printSqlIn…
d02d266 
…Parens to CompoundPredicate (apache#39064)

This PR enhances SQL expression handling by introducing a printSqlInParens flag to the CompoundPredicate class and setting it to true. This ensures that expressions in a CompoundPredicate are always enclosed in parentheses, similar to how it is handled in a BinaryPredicate. . By removing redundant handling of CompoundPredicate, the logic for handling SQL generation in the JdbcScanNode node has been simplified as the updated toSql method now handles this uniformly across expressions.
morningman pushed a commit that referenced this pull request Aug 14, 2024
@zy-kkk
[branch-2.1][fix](expr) Enhance SQL Expression Handling by Introducin…
c12137a 
…g printSqlInParens to CompoundPredicate (#39082)

pick #39064
dataroaring pushed a commit that referenced this pull request Aug 17, 2024
@zy-kkk @dataroaring
[fix](expr) Enhance SQL Expression Handling by Introducing printSqlIn…
8e3c030 
…Parens to CompoundPredicate (#39064)

This PR enhances SQL expression handling by introducing a printSqlInParens flag to the CompoundPredicate class and setting it to true. This ensures that expressions in a CompoundPredicate are always enclosed in parentheses, similar to how it is handled in a BinaryPredicate. . By removing redundant handling of CompoundPredicate, the logic for handling SQL generation in the JdbcScanNode node has been simplified as the updated toSql method now handles this uniformly across expressions.
@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

@morningman morningman morningman approved these changes

@CalvinKirs CalvinKirs CalvinKirs approved these changes

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

Successfully merging this pull request may close these issues.
5 participants
@zy-kkk @doris-robot @morningman @CalvinKirs @dataroaring