Skip to content

Conversation

@morrySnow
Copy link
Contributor

What problem does this PR solve?

related PR #58964

Problem Summary:

This pull request refactors how child physical plans are accessed in the ChildrenPropertiesRegulator class, simplifying the code and improving test clarity. The main change is the removal of the getChildPhysicalPlan helper method, replacing its usage with direct access to the plan from the children list. The tests are also updated to build child mocks more locally, improving test isolation and readability.

Refactoring and Simplification

  • Removed the getChildPhysicalPlan method from ChildrenPropertiesRegulator, and replaced its usage in visitPhysicalFilter and visitPhysicalProject with direct access to the child plan via children.get(0).getPlan(). This simplifies the code by eliminating unnecessary indirection. [1] [2] [3]

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

@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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17594	4172	4083	4083
q2	2056	361	240	240
q3	10143	1329	750	750
q4	10242	907	323	323
q5	7518	2238	1943	1943
q6	179	168	137	137
q7	1012	849	727	727
q8	9360	1462	1248	1248
q9	6997	5346	5328	5328
q10	6809	2398	1972	1972
q11	533	338	312	312
q12	683	722	580	580
q13	17752	3700	3028	3028
q14	291	302	286	286
q15	597	518	513	513
q16	710	681	633	633
q17	691	832	609	609
q18	7351	7702	7855	7702
q19	952	998	688	688
q20	442	406	255	255
q21	4544	4446	4134	4134
q22	1183	1066	1023	1023
Total cold run time: 107639 ms
Total hot run time: 36514 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4328	4224	4379	4224
q2	346	426	320	320
q3	2357	2775	2455	2455
q4	1393	1840	1394	1394
q5	4548	4622	4433	4433
q6	215	170	123	123
q7	2031	1954	1787	1787
q8	2730	2549	2666	2549
q9	7471	7496	7440	7440
q10	3132	3246	2814	2814
q11	611	523	495	495
q12	640	677	599	599
q13	3314	3663	3053	3053
q14	268	288	257	257
q15	528	500	481	481
q16	626	660	590	590
q17	1127	1303	1288	1288
q18	7449	7010	6958	6958
q19	862	796	849	796
q20	1916	1984	1787	1787
q21	4566	4278	4422	4278
q22	1074	1025	966	966
Total cold run time: 51532 ms
Total hot run time: 49087 ms

@doris-robot
Copy link

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

query5	4410	595	482	482
query6	329	234	234	234
query7	4215	459	271	271
query8	313	247	271	247
query9	8743	2565	2537	2537
query10	505	385	324	324
query11	15735	15033	14749	14749
query12	171	122	125	122
query13	1265	520	395	395
query14	5518	3002	2879	2879
query14_1	2688	2644	2674	2644
query15	219	196	185	185
query16	910	489	459	459
query17	970	729	606	606
query18	2437	447	346	346
query19	233	231	220	220
query20	120	117	117	117
query21	228	139	119	119
query22	3821	4108	3946	3946
query23	16588	16241	15922	15922
query23_1	16191	16146	15964	15964
query24	7381	1686	1253	1253
query24_1	1279	1215	1248	1215
query25	591	499	456	456
query26	1238	280	166	166
query27	2742	469	309	309
query28	4481	2144	2141	2141
query29	830	566	492	492
query30	323	245	223	223
query31	804	695	646	646
query32	85	76	71	71
query33	589	326	283	283
query34	916	925	525	525
query35	765	786	708	708
query36	845	924	822	822
query37	130	88	80	80
query38	3066	3037	2962	2962
query39	751	775	751	751
query39_1	714	697	703	697
query40	224	146	119	119
query41	67	63	63	63
query42	108	103	103	103
query43	428	423	410	410
query44	1305	753	733	733
query45	190	190	181	181
query46	875	964	611	611
query47	1664	1700	1633	1633
query48	335	336	243	243
query49	625	423	354	354
query50	662	300	219	219
query51	3838	3857	3779	3779
query52	113	111	97	97
query53	321	349	299	299
query54	290	269	275	269
query55	80	78	73	73
query56	295	290	299	290
query57	1165	1151	1075	1075
query58	274	262	280	262
query59	2378	2522	2349	2349
query60	315	325	283	283
query61	166	158	168	158
query62	756	720	675	675
query63	326	297	299	297
query64	5031	1302	1006	1006
query65	4020	3958	3954	3954
query66	1484	436	312	312
query67	15287	14910	14940	14910
query68	8345	988	716	716
query69	504	350	317	317
query70	1095	1027	998	998
query71	355	314	278	278
query72	6077	5043	5232	5043
query73	727	682	308	308
query74	8726	8960	8836	8836
query75	3193	3164	2795	2795
query76	3851	1133	745	745
query77	522	403	296	296
query78	9390	9492	8851	8851
query79	1396	866	618	618
query80	698	651	574	574
query81	495	270	231	231
query82	218	133	104	104
query83	278	253	239	239
query84	302	125	111	111
query85	921	503	470	470
query86	336	308	278	278
query87	3156	3413	3190	3190
query88	3618	2278	2279	2278
query89	469	421	398	398
query90	2248	163	157	157
query91	173	163	144	144
query92	92	74	64	64
query93	1247	941	552	552
query94	495	306	281	281
query95	573	335	304	304
query96	584	474	207	207
query97	2234	2315	2258	2258
query98	227	203	190	190
query99	1323	1474	1281	1281
Total cold run time: 259815 ms
Total hot run time: 179688 ms

@doris-robot
Copy link

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

query1	0.05	0.05	0.05
query2	0.09	0.05	0.05
query3	0.26	0.10	0.09
query4	1.61	0.11	0.11
query5	0.28	0.25	0.25
query6	1.16	0.67	0.64
query7	0.04	0.03	0.03
query8	0.06	0.04	0.04
query9	0.56	0.50	0.49
query10	0.55	0.56	0.54
query11	0.15	0.11	0.12
query12	0.14	0.11	0.12
query13	0.64	0.60	0.61
query14	0.99	0.98	0.99
query15	0.81	0.79	0.80
query16	0.43	0.39	0.40
query17	0.99	0.97	1.06
query18	0.24	0.21	0.21
query19	1.89	1.87	1.90
query20	0.02	0.01	0.01
query21	15.46	0.27	0.13
query22	4.66	0.05	0.04
query23	15.92	0.28	0.10
query24	1.59	0.70	0.28
query25	0.07	0.07	0.08
query26	0.15	0.13	0.13
query27	0.10	0.04	0.05
query28	4.91	1.21	1.03
query29	12.65	4.01	3.17
query30	0.27	0.14	0.12
query31	2.82	0.63	0.38
query32	3.24	0.55	0.45
query33	3.00	3.00	3.00
query34	16.76	5.27	4.51
query35	4.53	4.65	4.53
query36	0.66	0.49	0.48
query37	0.10	0.07	0.06
query38	0.08	0.04	0.04
query39	0.05	0.03	0.03
query40	0.16	0.14	0.13
query41	0.09	0.03	0.03
query42	0.05	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 98.32 s
Total hot run time: 27.14 s

@hello-stephen
Copy link
Contributor

FE UT Coverage Report

Increment line coverage 100.00% (2/2) 🎉
Increment coverage report
Complete coverage report

@hello-stephen
Copy link
Contributor

FE Regression Coverage Report

Increment line coverage 100.00% (2/2) 🎉
Increment coverage report
Complete coverage report

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants