Skip to content

[refactor](analysis) replace Expr.toSql() with visitor pattern#60901

Draft
morrySnow wants to merge 1 commit intoapache:masterfrom
morrySnow:ref-to-sql-claude
Draft

[refactor](analysis) replace Expr.toSql() with visitor pattern#60901
morrySnow wants to merge 1 commit intoapache:masterfrom
morrySnow:ref-to-sql-claude

Conversation

@morrySnow
Copy link
Contributor

What problem does this PR solve?

Remove toSql(), toSqlImpl(), toSqlWithoutTbl(), toExternalSql(), and toColumnLabel() from legacy Expr and all 39 subclasses. Introduce three visitor classes (ExprToSqlVisitor, ExprToExternalSqlVisitor, ExprToColumnLabelVisitor) plus ExprVisitor base and ToSqlParams context. Update all call sites across FE. Build and checkstyle pass clean.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

Remove toSql(), toSqlImpl(), toSqlWithoutTbl(), toExternalSql(), and
toColumnLabel() from legacy Expr and all 39 subclasses. Introduce three
visitor classes (ExprToSqlVisitor, ExprToExternalSqlVisitor,
ExprToColumnLabelVisitor) plus ExprVisitor base and ToSqlParams context.
Update all call sites across FE. Build and checkstyle pass clean.
@hello-stephen
Copy link
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@morrySnow
Copy link
Contributor Author

run buildall

@hello-stephen
Copy link
Contributor

FE UT Coverage Report

Increment line coverage 39.30% (268/682) 🎉
Increment coverage report
Complete coverage report

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
============================================
q1	17609	4603	4315	4315
q2	q3	10638	790	512	512
q4	4685	362	252	252
q5	7539	1201	1023	1023
q6	182	178	151	151
q7	784	870	676	676
q8	9305	1504	1353	1353
q9	4949	4753	4746	4746
q10	6853	1856	1656	1656
q11	449	261	249	249
q12	703	596	468	468
q13	17791	4235	3435	3435
q14	239	240	206	206
q15	939	812	796	796
q16	765	719	676	676
q17	739	873	427	427
q18	5994	5364	5303	5303
q19	1167	978	626	626
q20	509	504	386	386
q21	4954	2016	1506	1506
q22	363	311	248	248
Total cold run time: 97156 ms
Total hot run time: 29010 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4663	4551	4551	4551
q2	q3	1810	2224	1761	1761
q4	883	1241	788	788
q5	4070	4409	4322	4322
q6	186	181	144	144
q7	1907	1643	1506	1506
q8	2650	2724	2492	2492
q9	7567	7402	7468	7402
q10	2625	2815	2434	2434
q11	555	439	412	412
q12	506	585	453	453
q13	4078	4532	3687	3687
q14	284	292	295	292
q15	865	836	804	804
q16	697	778	690	690
q17	1151	1668	1364	1364
q18	7074	6950	6602	6602
q19	940	904	930	904
q20	2090	2267	2050	2050
q21	3938	3432	3442	3432
q22	456	429	416	416
Total cold run time: 48995 ms
Total hot run time: 46506 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 183538 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 1b42de1b429a493cc256fb54a29dd2ed0f50b51b, data reload: false

query5	4341	628	526	526
query6	341	225	215	215
query7	4212	475	277	277
query8	347	266	238	238
query9	8711	2779	2789	2779
query10	511	391	333	333
query11	16948	17537	17096	17096
query12	200	135	133	133
query13	1291	480	352	352
query14	6685	3317	3133	3133
query14_1	2961	2922	2919	2919
query15	214	199	182	182
query16	1018	507	470	470
query17	1211	760	664	664
query18	2983	432	329	329
query19	205	203	183	183
query20	140	135	126	126
query21	215	133	111	111
query22	4719	4855	4801	4801
query23	17167	16789	16611	16611
query23_1	16613	16738	16642	16642
query24	7167	1618	1205	1205
query24_1	1230	1226	1251	1226
query25	533	457	406	406
query26	1245	264	151	151
query27	2763	465	280	280
query28	4519	1865	1857	1857
query29	791	563	478	478
query30	312	246	207	207
query31	876	722	667	667
query32	80	74	74	74
query33	502	363	287	287
query34	918	912	560	560
query35	630	683	612	612
query36	1080	1130	902	902
query37	142	99	85	85
query38	2959	2966	2900	2900
query39	892	895	836	836
query39_1	808	826	831	826
query40	233	153	135	135
query41	69	61	57	57
query42	106	102	109	102
query43	382	382	358	358
query44	
query45	193	190	195	190
query46	894	1002	622	622
query47	2068	2103	2058	2058
query48	317	321	231	231
query49	644	447	385	385
query50	686	274	220	220
query51	4070	4018	4082	4018
query52	108	106	97	97
query53	292	341	293	293
query54	291	267	256	256
query55	92	84	79	79
query56	307	313	311	311
query57	1352	1349	1264	1264
query58	300	278	260	260
query59	2591	2606	2493	2493
query60	353	340	324	324
query61	150	151	145	145
query62	620	586	541	541
query63	309	276	282	276
query64	4916	1249	989	989
query65	
query66	1407	456	370	370
query67	16344	16263	16238	16238
query68	
query69	398	317	283	283
query70	1000	994	910	910
query71	340	314	300	300
query72	2835	2868	2566	2566
query73	548	560	323	323
query74	10069	9941	9779	9779
query75	2829	2769	2457	2457
query76	2289	1026	674	674
query77	364	399	312	312
query78	11267	11405	10694	10694
query79	1236	827	614	614
query80	1322	628	533	533
query81	574	277	244	244
query82	1016	155	117	117
query83	336	258	240	240
query84	246	115	100	100
query85	882	503	434	434
query86	409	310	277	277
query87	3089	3086	3009	3009
query88	3601	2688	2667	2667
query89	431	369	345	345
query90	1997	181	183	181
query91	173	175	135	135
query92	75	74	71	71
query93	1051	824	506	506
query94	650	324	293	293
query95	607	396	316	316
query96	634	526	234	234
query97	2476	2473	2391	2391
query98	239	219	213	213
query99	984	979	907	907
Total cold run time: 252911 ms
Total hot run time: 183538 ms

@morrySnow
Copy link
Contributor Author

run buildall

@hello-stephen
Copy link
Contributor

FE UT Coverage Report

Increment line coverage 39.30% (268/682) 🎉
Increment coverage report
Complete coverage report

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
============================================
q1	17669	4592	4299	4299
q2	q3	10638	828	536	536
q4	4702	357	249	249
q5	7772	1202	1018	1018
q6	200	194	151	151
q7	807	849	657	657
q8	10425	1472	1355	1355
q9	5932	4787	4663	4663
q10	6855	1904	1641	1641
q11	467	247	243	243
q12	749	564	467	467
q13	17801	4231	3422	3422
q14	240	236	214	214
q15	938	809	803	803
q16	763	708	693	693
q17	726	882	418	418
q18	6519	5419	5227	5227
q19	1250	973	618	618
q20	509	622	532	532
q21	4508	1958	1439	1439
q22	358	320	269	269
Total cold run time: 99828 ms
Total hot run time: 28914 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4758	4726	4576	4576
q2	q3	1802	2206	1792	1792
q4	861	1173	774	774
q5	4017	4377	4432	4377
q6	187	188	147	147
q7	1815	1750	1519	1519
q8	2511	2748	2524	2524
q9	7635	7450	7342	7342
q10	2611	2828	2399	2399
q11	568	508	452	452
q12	502	589	457	457
q13	3918	4689	3565	3565
q14	286	296	276	276
q15	904	845	821	821
q16	737	791	709	709
q17	1182	1506	1423	1423
q18	7214	6831	6657	6657
q19	942	906	935	906
q20	2083	2164	1986	1986
q21	3971	3483	3393	3393
q22	459	448	386	386
Total cold run time: 48963 ms
Total hot run time: 46481 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 183986 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 1b42de1b429a493cc256fb54a29dd2ed0f50b51b, data reload: false

query5	4334	646	526	526
query6	327	220	202	202
query7	4209	469	275	275
query8	352	244	231	231
query9	8731	2751	2753	2751
query10	506	379	336	336
query11	16962	17531	17208	17208
query12	192	129	122	122
query13	1309	463	352	352
query14	5873	3315	3081	3081
query14_1	2905	2847	2969	2847
query15	206	196	178	178
query16	971	507	562	507
query17	2032	740	622	622
query18	2547	457	341	341
query19	201	216	181	181
query20	136	144	137	137
query21	224	150	135	135
query22	5326	4896	4968	4896
query23	17171	16892	16484	16484
query23_1	16681	16721	16740	16721
query24	7215	1645	1242	1242
query24_1	1215	1234	1176	1176
query25	542	449	402	402
query26	1107	254	154	154
query27	2769	482	284	284
query28	4510	1874	1903	1874
query29	800	555	465	465
query30	304	249	206	206
query31	857	722	641	641
query32	79	73	70	70
query33	511	340	277	277
query34	900	902	557	557
query35	610	663	592	592
query36	1092	1147	913	913
query37	131	99	86	86
query38	2922	2888	2859	2859
query39	907	865	826	826
query39_1	819	840	823	823
query40	236	155	137	137
query41	62	62	57	57
query42	105	104	100	100
query43	378	392	358	358
query44	
query45	206	192	182	182
query46	871	991	603	603
query47	2089	2159	2067	2067
query48	320	316	238	238
query49	639	463	379	379
query50	684	275	218	218
query51	4151	4085	4081	4081
query52	104	108	98	98
query53	285	336	275	275
query54	300	278	259	259
query55	86	79	79	79
query56	303	300	323	300
query57	1373	1347	1260	1260
query58	295	286	286	286
query59	2612	2666	2531	2531
query60	348	346	335	335
query61	174	172	175	172
query62	637	603	552	552
query63	323	284	278	278
query64	4860	1350	1112	1112
query65	
query66	1455	467	375	375
query67	16323	16402	16347	16347
query68	
query69	424	325	312	312
query70	985	946	940	940
query71	340	307	316	307
query72	2993	2877	2483	2483
query73	540	565	317	317
query74	10187	9921	9726	9726
query75	2859	2723	2454	2454
query76	2381	1025	677	677
query77	364	385	319	319
query78	11320	11559	10821	10821
query79	2301	796	591	591
query80	1681	631	548	548
query81	573	295	245	245
query82	993	150	120	120
query83	345	272	248	248
query84	248	118	103	103
query85	885	498	439	439
query86	419	297	296	296
query87	3103	3101	3020	3020
query88	3626	2692	2670	2670
query89	435	371	344	344
query90	1989	176	183	176
query91	168	165	133	133
query92	80	77	72	72
query93	1239	897	507	507
query94	628	327	303	303
query95	582	334	382	334
query96	645	538	231	231
query97	2479	2519	2416	2416
query98	238	222	216	216
query99	980	992	921	921
Total cold run time: 255176 ms
Total hot run time: 183986 ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants