Skip to content

branch-3.0: [fix](Nereids) deep copy for LogicalWindow is wrong #48861 #49013

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
Merged
dataroaring merged 1 commit into from
Mar 20, 2025
Merged

branch-3.0: [fix](Nereids) deep copy for LogicalWindow is wrong #48861 #49013

dataroaring merged 1 commit into from
Mar 20, 2025

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Mar 13, 2025

Cherry-picked from #48861

@github-actions github-actions bot requested a review from dataroaring as a code owner March 13, 2025 04:23
@Thearas
Copy link
Contributor

Thearas commented Mar 13, 2025

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?

@dataroaring dataroaring reopened this Mar 13, 2025
@Thearas
Copy link
Contributor

Thearas commented Mar 13, 2025

run buildall

@morrySnow
[fix](Nereids) deep copy for LogicalWindow is wrong (#48861)
b95a1d0 
Related PR: #21727 #14397

Problem Summary:

1. forgot to copy isChecked flag in LogicalWindow when do deep copy
2. implement LogicalWindow To PhyscialWindow should not check isChecked
flag

This PR:
1. check deep copy for all plan node
2. remove check isChecked in LogicalWindow To PhyscialWindow
@morrySnow morrySnow force-pushed the auto-pick-48861-branch-3.0 branch from caa6feb to b95a1d0 Compare March 13, 2025 08:15
@morrySnow
Copy link
Contributor

morrySnow commented Mar 13, 2025

run buildall

1 similar comment
@morrySnow
Copy link
Contributor

morrySnow commented Mar 14, 2025

run buildall

@doris-robot
Copy link

doris-robot commented Mar 14, 2025

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

------ Round 1 ----------------------------------
q1	17604	6854	6586	6586
q2	2066	190	177	177
q3	10615	1076	1232	1076
q4	10567	710	729	710
q5	7757	2828	3038	2828
q6	221	138	140	138
q7	949	609	613	609
q8	9364	1916	1998	1916
q9	6555	6381	6419	6381
q10	7036	2291	2293	2291
q11	474	261	270	261
q12	405	217	204	204
q13	17780	2952	3005	2952
q14	235	209	211	209
q15	511	463	472	463
q16	685	603	586	586
q17	968	552	556	552
q18	7335	6747	6683	6683
q19	1399	1088	1146	1088
q20	475	200	199	199
q21	3969	3229	2959	2959
q22	1102	994	964	964
Total cold run time: 108072 ms
Total hot run time: 39832 ms

----- Round 2, with runtime_filter_mode=off -----
q1	6602	6569	6561	6561
q2	325	232	230	230
q3	2864	2783	2930	2783
q4	2014	1782	1752	1752
q5	5768	5755	5704	5704
q6	202	126	128	126
q7	2231	1804	1829	1804
q8	3350	3546	3529	3529
q9	8996	8782	8902	8782
q10	3565	3531	3482	3482
q11	587	486	495	486
q12	829	588	604	588
q13	10354	3251	3186	3186
q14	316	272	275	272
q15	525	473	460	460
q16	693	649	662	649
q17	1860	1599	1648	1599
q18	8173	7938	7782	7782
q19	1673	1540	1471	1471
q20	2101	1869	1829	1829
q21	5672	5135	5346	5135
q22	1175	1018	1052	1018
Total cold run time: 69875 ms
Total hot run time: 59228 ms

@doris-robot
Copy link

doris-robot commented Mar 14, 2025

TPC-DS: Total hot run time: 197473 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 b95a1d094271e3210543de954c6f03653bba32a0, data reload: false

query1	1315	904	895	895
query2	6232	1978	1963	1963
query3	10858	4239	4258	4239
query4	65885	29047	23387	23387
query5	4968	452	452	452
query6	428	191	179	179
query7	5681	325	313	313
query8	314	222	222	222
query9	9123	2624	2640	2624
query10	469	253	247	247
query11	17506	15205	15746	15205
query12	154	103	102	102
query13	1545	424	436	424
query14	10391	7644	7583	7583
query15	203	173	184	173
query16	7274	440	510	440
query17	1103	594	583	583
query18	1903	336	337	336
query19	224	160	162	160
query20	127	121	115	115
query21	207	106	100	100
query22	4802	4346	4351	4346
query23	34230	33990	33892	33892
query24	6122	2819	2943	2819
query25	541	425	442	425
query26	658	170	169	169
query27	1997	384	368	368
query28	4246	2499	2446	2446
query29	735	467	473	467
query30	246	169	160	160
query31	1012	837	847	837
query32	74	56	72	56
query33	499	296	294	294
query34	925	543	509	509
query35	867	707	727	707
query36	1091	981	990	981
query37	119	63	73	63
query38	4121	4012	4009	4009
query39	1525	1488	1478	1478
query40	203	102	100	100
query41	54	49	48	48
query42	108	97	102	97
query43	524	502	493	493
query44	1216	820	848	820
query45	181	172	165	165
query46	1147	723	730	723
query47	1990	1933	1921	1921
query48	490	391	425	391
query49	734	400	389	389
query50	833	435	418	418
query51	7242	7200	7140	7140
query52	105	90	87	87
query53	250	176	176	176
query54	543	443	449	443
query55	76	79	74	74
query56	250	264	251	251
query57	1257	1099	1108	1099
query58	203	197	205	197
query59	3119	3022	2907	2907
query60	266	251	242	242
query61	114	106	103	103
query62	764	673	671	671
query63	219	184	184	184
query64	1389	667	639	639
query65	3249	3170	3209	3170
query66	710	299	293	293
query67	15952	15555	15520	15520
query68	4032	591	585	585
query69	428	268	259	259
query70	1156	1133	1138	1133
query71	341	255	248	248
query72	6461	4140	4150	4140
query73	759	344	361	344
query74	10244	9345	9278	9278
query75	3346	2691	2668	2668
query76	1885	1057	1014	1014
query77	521	272	266	266
query78	10589	9636	9569	9569
query79	1293	590	591	590
query80	819	441	438	438
query81	517	240	232	232
query82	468	89	84	84
query83	166	145	145	145
query84	289	80	79	79
query85	836	304	298	298
query86	327	300	298	298
query87	4342	4264	4225	4225
query88	3703	2406	2358	2358
query89	409	290	285	285
query90	2049	179	186	179
query91	186	149	149	149
query92	64	50	50	50
query93	1419	569	562	562
query94	818	301	295	295
query95	359	255	257	255
query96	605	288	282	282
query97	3320	3159	3175	3159
query98	213	211	202	202
query99	1708	1275	1295	1275
Total cold run time: 316580 ms
Total hot run time: 197473 ms

@doris-robot
Copy link

doris-robot commented Mar 14, 2025

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

query1	0.03	0.03	0.04
query2	0.06	0.03	0.03
query3	0.23	0.06	0.06
query4	1.62	0.10	0.10
query5	0.52	0.51	0.50
query6	1.14	0.72	0.72
query7	0.02	0.01	0.03
query8	0.04	0.03	0.03
query9	0.59	0.53	0.51
query10	0.57	0.57	0.57
query11	0.15	0.11	0.11
query12	0.15	0.11	0.11
query13	0.61	0.61	0.61
query14	2.74	2.77	2.85
query15	0.89	0.82	0.84
query16	0.38	0.38	0.38
query17	1.05	1.06	1.09
query18	0.24	0.23	0.22
query19	1.85	1.89	2.01
query20	0.01	0.01	0.01
query21	15.35	0.61	0.59
query22	2.72	2.10	2.02
query23	17.00	0.99	0.78
query24	3.46	1.38	0.46
query25	0.29	0.15	0.06
query26	0.47	0.14	0.15
query27	0.05	0.05	0.06
query28	10.37	0.51	0.44
query29	12.62	3.19	3.16
query30	0.25	0.06	0.06
query31	2.85	0.40	0.39
query32	3.23	0.46	0.47
query33	2.98	3.04	3.02
query34	17.18	4.44	4.47
query35	4.55	4.47	4.53
query36	0.68	0.47	0.49
query37	0.10	0.07	0.06
query38	0.04	0.03	0.04
query39	0.04	0.02	0.03
query40	0.16	0.12	0.13
query41	0.08	0.03	0.02
query42	0.03	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 107.43 s
Total hot run time: 31.85 s

dataroaring
dataroaring approved these changes Mar 20, 2025
View reviewed changes
Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

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

LGTM

@dataroaring dataroaring merged commit 67d6e03 into branch-3.0 Mar 20, 2025
26 of 29 checks passed
@github-actions github-actions bot deleted the auto-pick-48861-branch-3.0 branch March 20, 2025 06:48
Jibing-Li added a commit to Jibing-Li/incubator-doris that referenced this pull request May 15, 2025
@Jibing-Li
Revert "branch-3.0: [fix](Nereids) deep copy for LogicalWindow is wrong
2d7d173 
apache#48861 (apache#49013)"

This reverts commit 67d6e03.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dataroaring dataroaring dataroaring approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Thearas @morrySnow @doris-robot @dataroaring