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

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