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](nereids) fix bug in PhysicalTopN.equals() #46547

Merged
merged 2 commits into from
Jan 8, 2025
Merged

[fix](nereids) fix bug in PhysicalTopN.equals() #46547

merged 2 commits into from
Jan 8, 2025

Conversation

englefly
Copy link
Contributor

@englefly englefly commented Jan 7, 2025

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

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

@Thearas
Copy link
Contributor

Thearas commented Jan 7, 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?

@englefly
Copy link
Contributor Author

englefly commented Jan 7, 2025

run buildall

morrySnow
morrySnow reviewed Jan 7, 2025
View reviewed changes
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalTopN.java
@@ -92,7 +92,9 @@ public boolean equals(Object o) {
return false;
}
PhysicalTopN<?> that = (PhysicalTopN<?>) o;
return limit == that.limit && offset == that.offset;
return limit == that.limit && offset == that.offset
Copy link
Contributor

Choose a reason for hiding this comment

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

add a ut to test equals

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17583	6124	6049	6049
q2	2043	315	176	176
q3	10474	1242	745	745
q4	10220	870	434	434
q5	7580	2191	1993	1993
q6	205	176	148	148
q7	912	750	630	630
q8	9258	1373	1222	1222
q9	5170	4852	4894	4852
q10	6760	2301	1839	1839
q11	487	285	272	272
q12	352	359	230	230
q13	17769	3661	3017	3017
q14	235	237	222	222
q15	554	497	512	497
q16	646	603	586	586
q17	588	858	345	345
q18	7412	6455	6624	6455
q19	1229	956	578	578
q20	327	330	192	192
q21	2843	2300	2013	2013
q22	367	336	325	325
Total cold run time: 103014 ms
Total hot run time: 32820 ms

----- Round 2, with runtime_filter_mode=off -----
q1	6211	6219	6214	6214
q2	239	327	237	237
q3	2245	2674	2303	2303
q4	1460	1892	1434	1434
q5	4361	4754	4901	4754
q6	186	173	144	144
q7	2131	2002	1857	1857
q8	2580	2801	2678	2678
q9	7298	7107	7229	7107
q10	3078	3310	2788	2788
q11	591	517	506	506
q12	666	765	623	623
q13	3497	3854	3260	3260
q14	281	306	279	279
q15	578	509	509	509
q16	645	685	637	637
q17	1191	1721	1275	1275
q18	7944	7362	7121	7121
q19	849	1093	1037	1037
q20	1861	1975	1903	1903
q21	5482	5064	4909	4909
q22	616	611	539	539
Total cold run time: 53990 ms
Total hot run time: 52114 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 190455 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 6d3e113f2553180a4e56609bb095d5d19ca5a735, data reload: false

query1	975	374	375	374
query2	6562	2289	2358	2289
query3	6707	214	211	211
query4	34004	23593	23645	23593
query5	4365	601	462	462
query6	294	227	185	185
query7	4650	486	304	304
query8	309	249	244	244
query9	9705	2779	2769	2769
query10	471	310	255	255
query11	18096	15609	15506	15506
query12	172	112	106	106
query13	1694	533	412	412
query14	11520	7645	6821	6821
query15	248	206	194	194
query16	8208	612	434	434
query17	1591	745	589	589
query18	2127	430	296	296
query19	207	170	142	142
query20	116	111	105	105
query21	209	139	102	102
query22	4213	4391	4238	4238
query23	34297	33486	33456	33456
query24	6243	2303	2266	2266
query25	514	445	372	372
query26	1205	273	159	159
query27	2039	468	341	341
query28	5451	2479	2450	2450
query29	727	556	406	406
query30	227	176	148	148
query31	968	915	831	831
query32	98	61	61	61
query33	507	376	294	294
query34	757	828	503	503
query35	808	815	720	720
query36	1015	1054	935	935
query37	123	97	74	74
query38	4261	4241	4126	4126
query39	1500	1440	1403	1403
query40	205	118	99	99
query41	47	46	47	46
query42	125	109	100	100
query43	510	521	495	495
query44	1340	789	793	789
query45	176	181	161	161
query46	876	1036	637	637
query47	1886	1888	1878	1878
query48	386	391	337	337
query49	789	470	409	409
query50	631	631	386	386
query51	7094	7123	6875	6875
query52	108	99	94	94
query53	223	254	189	189
query54	486	470	413	413
query55	82	81	79	79
query56	249	256	249	249
query57	1193	1187	1134	1134
query58	253	223	221	221
query59	3092	3057	2944	2944
query60	276	269	250	250
query61	113	135	105	105
query62	873	797	721	721
query63	236	194	190	190
query64	4505	1002	643	643
query65	3246	3195	3221	3195
query66	1045	432	313	313
query67	15930	15820	15438	15438
query68	7801	702	514	514
query69	458	290	251	251
query70	1233	1149	1154	1149
query71	431	301	259	259
query72	6091	3840	3900	3840
query73	665	744	359	359
query74	9987	9369	9010	9010
query75	3911	3159	2648	2648
query76	3749	1179	764	764
query77	777	358	286	286
query78	10019	10651	9427	9427
query79	3118	788	599	599
query80	614	552	437	437
query81	484	281	232	232
query82	544	157	137	137
query83	167	168	147	147
query84	245	86	71	71
query85	787	368	300	300
query86	399	331	314	314
query87	4593	4434	4448	4434
query88	4698	2234	2225	2225
query89	409	334	296	296
query90	1888	190	187	187
query91	132	137	105	105
query92	69	58	51	51
query93	1153	828	534	534
query94	653	406	297	297
query95	334	261	251	251
query96	477	603	290	290
query97	2902	2973	2853	2853
query98	223	198	200	198
query99	1736	1593	1424	1424
Total cold run time: 293905 ms
Total hot run time: 190455 ms

@doris-robot
Copy link

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

query1	0.03	0.04	0.03
query2	0.07	0.04	0.03
query3	0.23	0.08	0.07
query4	1.61	0.11	0.11
query5	0.43	0.42	0.41
query6	1.17	0.64	0.66
query7	0.02	0.01	0.01
query8	0.04	0.03	0.03
query9	0.58	0.49	0.51
query10	0.55	0.58	0.55
query11	0.14	0.11	0.10
query12	0.13	0.10	0.10
query13	0.60	0.61	0.60
query14	2.72	2.75	2.77
query15	0.90	0.83	0.82
query16	0.38	0.38	0.38
query17	1.02	1.02	1.04
query18	0.22	0.22	0.21
query19	1.88	1.93	1.91
query20	0.02	0.01	0.01
query21	15.38	0.96	0.57
query22	0.74	0.83	0.64
query23	15.30	1.44	0.54
query24	2.94	1.39	1.41
query25	0.22	0.15	0.06
query26	0.18	0.16	0.14
query27	0.07	0.05	0.04
query28	14.47	1.47	1.04
query29	12.54	4.10	3.39
query30	0.26	0.08	0.06
query31	2.83	0.58	0.38
query32	3.23	0.54	0.46
query33	3.20	3.09	3.07
query34	16.42	5.13	4.50
query35	4.48	4.46	4.47
query36	0.82	0.48	0.48
query37	0.10	0.06	0.06
query38	0.04	0.04	0.04
query39	0.03	0.02	0.03
query40	0.18	0.14	0.14
query41	0.08	0.02	0.02
query42	0.04	0.02	0.03
query43	0.04	0.03	0.02
Total cold run time: 106.33 s
Total hot run time: 31.81 s

@morrySnow morrySnow added not-merge/2.0 do not merge into 2.0 branch dev/2.1.x dev/3.0.x and removed not-merge/2.0 do not merge into 2.0 branch labels Jan 7, 2025
@englefly englefly changed the title [fix](nereids) fix bug in PhysicalTopN.equal() [fix](nereids) fix bug in PhysicalTopN.equals() Jan 8, 2025
@englefly
Copy link
Contributor Author

englefly commented Jan 8, 2025

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17573	6136	6094	6094
q2	2052	298	171	171
q3	10429	1239	761	761
q4	10198	887	460	460
q5	7499	2237	2032	2032
q6	210	188	150	150
q7	1003	788	638	638
q8	9232	1411	1211	1211
q9	5360	5028	4962	4962
q10	6777	2341	1876	1876
q11	493	286	265	265
q12	352	383	226	226
q13	17766	3672	3167	3167
q14	235	241	224	224
q15	569	502	490	490
q16	644	609	608	608
q17	591	871	328	328
q18	7069	6461	6414	6414
q19	2662	988	585	585
q20	306	324	189	189
q21	3152	2237	2088	2088
q22	364	339	306	306
Total cold run time: 104536 ms
Total hot run time: 33245 ms

----- Round 2, with runtime_filter_mode=off -----
q1	6343	6866	6253	6253
q2	233	328	242	242
q3	2291	2678	2297	2297
q4	1469	1840	1396	1396
q5	4372	4801	4956	4801
q6	184	185	147	147
q7	2090	1974	1830	1830
q8	2675	2830	2804	2804
q9	7363	7357	7404	7357
q10	3088	3381	2747	2747
q11	563	513	503	503
q12	634	779	654	654
q13	3534	3879	3330	3330
q14	301	306	277	277
q15	572	539	508	508
q16	636	678	654	654
q17	1246	1751	1280	1280
q18	7897	7442	7398	7398
q19	915	1226	1128	1128
q20	2006	2059	1951	1951
q21	5754	5095	4997	4997
q22	612	627	593	593
Total cold run time: 54778 ms
Total hot run time: 53147 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 197193 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 8ce0dc48bbfea1ce2ef56be0d4eebfc2745c98dc, data reload: false

query1	1294	984	932	932
query2	6361	2369	2308	2308
query3	11067	4773	4844	4773
query4	33287	23885	23483	23483
query5	3858	594	465	465
query6	273	199	200	199
query7	3980	480	297	297
query8	289	250	225	225
query9	9510	2792	2797	2792
query10	457	320	240	240
query11	17876	15328	15117	15117
query12	149	111	105	105
query13	1573	563	402	402
query14	10186	6884	7442	6884
query15	223	218	197	197
query16	8177	597	451	451
query17	1543	779	627	627
query18	2132	413	325	325
query19	232	199	177	177
query20	118	117	114	114
query21	210	137	110	110
query22	4659	4504	4398	4398
query23	34547	33881	33672	33672
query24	6487	2303	2355	2303
query25	489	457	394	394
query26	778	276	152	152
query27	2041	495	349	349
query28	5476	2549	2539	2539
query29	636	547	411	411
query30	212	184	144	144
query31	995	934	810	810
query32	76	59	59	59
query33	499	351	314	314
query34	773	851	512	512
query35	856	821	746	746
query36	1052	1080	962	962
query37	123	99	81	81
query38	4392	4371	4237	4237
query39	1507	1472	1507	1472
query40	209	118	102	102
query41	49	44	44	44
query42	133	98	103	98
query43	511	523	494	494
query44	1412	832	849	832
query45	177	173	177	173
query46	894	1060	669	669
query47	2031	2029	1957	1957
query48	411	419	330	330
query49	732	504	407	407
query50	669	660	388	388
query51	7025	7190	7165	7165
query52	99	100	94	94
query53	229	247	201	201
query54	482	498	427	427
query55	86	81	75	75
query56	259	264	250	250
query57	1258	1240	1184	1184
query58	245	229	226	226
query59	3167	3264	3314	3264
query60	284	278	242	242
query61	109	110	124	110
query62	854	819	769	769
query63	229	191	185	185
query64	3415	1050	667	667
query65	3324	3228	3327	3228
query66	890	418	318	318
query67	16637	15942	15573	15573
query68	8499	688	527	527
query69	492	308	250	250
query70	1226	1038	1125	1038
query71	438	301	261	261
query72	6507	3927	3861	3861
query73	659	749	365	365
query74	10656	9290	9070	9070
query75	4453	3115	2599	2599
query76	4050	1187	753	753
query77	762	369	287	287
query78	10089	10031	9589	9589
query79	3222	833	578	578
query80	746	519	420	420
query81	468	260	222	222
query82	332	153	131	131
query83	190	157	139	139
query84	284	90	67	67
query85	737	343	307	307
query86	352	307	311	307
query87	4659	4551	4444	4444
query88	3433	2307	2249	2249
query89	432	335	289	289
query90	1996	189	190	189
query91	134	133	107	107
query92	62	54	51	51
query93	1847	893	527	527
query94	649	395	291	291
query95	336	266	247	247
query96	492	610	282	282
query97	2875	3017	2856	2856
query98	222	197	200	197
query99	1658	1552	1443	1443
Total cold run time: 297170 ms
Total hot run time: 197193 ms

@doris-robot
Copy link

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

query1	0.04	0.05	0.04
query2	0.07	0.03	0.03
query3	0.24	0.07	0.07
query4	1.64	0.10	0.10
query5	0.42	0.43	0.43
query6	1.18	0.65	0.66
query7	0.02	0.02	0.01
query8	0.04	0.04	0.04
query9	0.58	0.49	0.51
query10	0.55	0.55	0.55
query11	0.15	0.11	0.10
query12	0.14	0.10	0.11
query13	0.60	0.58	0.60
query14	2.73	2.78	2.82
query15	0.90	0.83	0.83
query16	0.39	0.41	0.38
query17	1.06	1.04	1.02
query18	0.22	0.21	0.22
query19	1.94	1.88	1.94
query20	0.01	0.02	0.02
query21	15.36	0.94	0.58
query22	0.75	0.72	0.71
query23	15.36	1.44	0.56
query24	2.87	1.70	1.99
query25	0.20	0.18	0.14
query26	0.22	0.15	0.14
query27	0.06	0.05	0.04
query28	14.42	1.49	1.06
query29	12.56	3.94	3.28
query30	0.26	0.09	0.06
query31	2.83	0.57	0.37
query32	3.23	0.52	0.46
query33	3.13	3.00	3.09
query34	16.98	5.17	4.56
query35	4.55	4.51	4.55
query36	0.84	0.47	0.50
query37	0.09	0.07	0.06
query38	0.04	0.03	0.03
query39	0.04	0.02	0.02
query40	0.16	0.13	0.12
query41	0.07	0.02	0.03
query42	0.04	0.02	0.02
query43	0.03	0.03	0.03
Total cold run time: 107.01 s
Total hot run time: 32.25 s

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 8, 2025

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 Jan 8, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 8, 2025

PR approved by anyone and no changes requested.

yujun777
yujun777 approved these changes Jan 8, 2025
View reviewed changes
Copy link
Collaborator

@yujun777 yujun777 left a comment

Choose a reason for hiding this comment

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

LGTM

@morrySnow morrySnow merged commit 57e0a1c into apache:master Jan 8, 2025
26 of 28 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 8, 2025
@englefly
[fix](nereids) fix bug in PhysicalTopN.equals() (#46547)
7293a7e
@github-actions github-actions bot mentioned this pull request Jan 8, 2025
Merged
github-actions bot pushed a commit that referenced this pull request Jan 8, 2025
@englefly
[fix](nereids) fix bug in PhysicalTopN.equals() (#46547)
cee9ddc
@github-actions github-actions bot mentioned this pull request Jan 8, 2025
Merged
yiguolei pushed a commit that referenced this pull request Jan 8, 2025
@englefly @yiguolei
[fix](nereids) fix bug in PhysicalTopN.equals() (#46547)
5a9976e
yiguolei pushed a commit that referenced this pull request Jan 9, 2025
@github-actions @englefly
branch-2.1: [fix](nereids) fix bug in PhysicalTopN.equals() #46547 (#…
afeff5d 
…46633)

Cherry-picked from #46547

Co-authored-by: minghong <zhouminghong@selectdb.com>
dataroaring pushed a commit that referenced this pull request Feb 24, 2025
@github-actions @englefly
branch-3.0: [fix](nereids) fix bug in PhysicalTopN.equals() #46547 (#…
0311659 
…46632)

Cherry-picked from #46547

Co-authored-by: minghong <zhouminghong@selectdb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yujun777 yujun777 yujun777 approved these changes

@morrySnow morrySnow morrySnow approved these changes

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

Successfully merging this pull request may close these issues.
7 participants
@englefly @Thearas @doris-robot @yujun777 @morrySnow @wm1581066 @dataroaring