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 logical repeat computefd #47321

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[fix](nereids)fix logical repeat computefd #47321

wants to merge 2 commits into from
+21 −1

Conversation

feiniaofeiafei
Copy link
Contributor

@feiniaofeiafei feiniaofeiafei commented Jan 22, 2025 •
edited
Loading

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:
If a child of LogicalRepeat has an output slot that satisfies the "uniform and not null" condition, this slot is determined by other output slots. However, since LogicalRepeat introduces null values, the previously "uniform and not null" slot will no longer satisfy this condition. As a result, the functional dependency (FD) derived based on "uniform and not null" should not be preserved. Therefore, LogicalRepeat should not propagate this FD upwards.

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?

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17591	5503	5391	5391
q2	2051	302	169	169
q3	10671	1274	751	751
q4	10553	951	519	519
q5	9543	2351	2133	2133
q6	204	170	137	137
q7	900	765	596	596
q8	9241	1362	1173	1173
q9	5147	4870	4907	4870
q10	6854	2311	1882	1882
q11	482	282	254	254
q12	346	367	222	222
q13	17760	3692	3062	3062
q14	234	233	213	213
q15	522	477	455	455
q16	630	631	583	583
q17	562	867	324	324
q18	7114	6517	6231	6231
q19	2191	956	572	572
q20	315	312	188	188
q21	2889	2206	1991	1991
q22	368	333	326	326
Total cold run time: 106168 ms
Total hot run time: 32042 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5665	5496	5507	5496
q2	238	332	239	239
q3	2262	2632	2333	2333
q4	1375	1838	1414	1414
q5	4295	4730	4747	4730
q6	168	164	130	130
q7	2055	1899	1815	1815
q8	2631	2760	2715	2715
q9	7220	7249	7381	7249
q10	2995	3328	2693	2693
q11	566	511	485	485
q12	685	783	614	614
q13	3486	3892	3338	3338
q14	284	299	276	276
q15	517	470	473	470
q16	657	677	664	664
q17	1240	1722	1267	1267
q18	7731	7562	7328	7328
q19	829	985	1161	985
q20	1998	2101	1850	1850
q21	5843	5048	5180	5048
q22	610	659	606	606
Total cold run time: 53350 ms
Total hot run time: 51745 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 194183 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 988b7dfb482433e5084814132d7ed4d02b307bc4, data reload: false

query1	1313	951	952	951
query2	6187	2138	2124	2124
query3	11007	4530	4323	4323
query4	60449	29208	22965	22965
query5	5611	602	446	446
query6	434	196	197	196
query7	5575	509	289	289
query8	334	237	236	236
query9	8573	2735	2694	2694
query10	467	306	247	247
query11	17323	15102	15842	15102
query12	172	111	108	108
query13	1479	547	426	426
query14	10531	7147	7016	7016
query15	207	211	192	192
query16	7252	641	491	491
query17	1130	704	565	565
query18	1902	399	316	316
query19	201	173	150	150
query20	116	113	138	113
query21	211	120	101	101
query22	4342	4745	4481	4481
query23	34131	33439	33500	33439
query24	5472	2277	2389	2277
query25	449	462	405	405
query26	648	282	158	158
query27	1619	465	331	331
query28	3977	2513	2490	2490
query29	539	581	416	416
query30	211	184	158	158
query31	939	897	791	791
query32	68	64	56	56
query33	472	367	296	296
query34	742	878	514	514
query35	831	846	775	775
query36	1001	1078	969	969
query37	129	104	84	84
query38	4364	4434	4374	4374
query39	1517	1425	1423	1423
query40	216	111	101	101
query41	51	55	48	48
query42	127	114	105	105
query43	538	541	518	518
query44	1336	832	821	821
query45	176	175	206	175
query46	887	1051	651	651
query47	1901	1902	1834	1834
query48	394	429	319	319
query49	718	496	437	437
query50	668	670	398	398
query51	7127	6983	7045	6983
query52	101	100	91	91
query53	225	254	189	189
query54	480	500	415	415
query55	84	99	77	77
query56	254	256	252	252
query57	1263	1190	1094	1094
query58	238	229	238	229
query59	2960	3114	2981	2981
query60	280	273	254	254
query61	119	112	119	112
query62	716	712	668	668
query63	227	187	182	182
query64	1257	1036	677	677
query65	3268	3160	3158	3158
query66	707	388	293	293
query67	16048	15871	15494	15494
query68	5010	841	528	528
query69	497	310	261	261
query70	1232	1142	1175	1142
query71	411	287	310	287
query72	6093	3823	3814	3814
query73	832	743	371	371
query74	10405	9015	8803	8803
query75	3211	3146	2656	2656
query76	3799	1160	766	766
query77	490	354	272	272
query78	10133	10215	9259	9259
query79	2727	794	609	609
query80	651	625	429	429
query81	518	282	238	238
query82	286	146	116	116
query83	170	174	152	152
query84	284	95	73	73
query85	751	355	307	307
query86	381	294	287	287
query87	4379	4508	4358	4358
query88	4541	2192	2183	2183
query89	388	332	283	283
query90	1668	188	190	188
query91	139	134	108	108
query92	67	56	53	53
query93	2782	880	529	529
query94	678	400	304	304
query95	335	260	253	253
query96	492	617	280	280
query97	2825	2845	2748	2748
query98	229	199	203	199
query99	1318	1364	1283	1283
Total cold run time: 312013 ms
Total hot run time: 194183 ms

@doris-robot
Copy link

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

query1	0.03	0.04	0.03
query2	0.08	0.03	0.03
query3	0.24	0.07	0.07
query4	1.62	0.11	0.11
query5	0.41	0.41	0.41
query6	1.15	0.64	0.64
query7	0.03	0.02	0.01
query8	0.04	0.03	0.04
query9	0.61	0.51	0.49
query10	0.55	0.56	0.57
query11	0.13	0.10	0.10
query12	0.14	0.10	0.11
query13	0.62	0.60	0.60
query14	2.72	2.75	2.84
query15	0.90	0.83	0.82
query16	0.36	0.39	0.38
query17	0.98	1.05	1.01
query18	0.24	0.22	0.21
query19	1.88	1.88	2.00
query20	0.01	0.02	0.01
query21	15.35	0.90	0.56
query22	0.76	0.80	0.67
query23	15.24	1.46	0.54
query24	3.15	0.91	1.46
query25	0.28	0.14	0.13
query26	0.21	0.15	0.14
query27	0.05	0.06	0.04
query28	13.75	0.99	0.43
query29	12.57	3.99	3.32
query30	0.25	0.09	0.06
query31	2.85	0.59	0.38
query32	3.22	0.55	0.46
query33	2.99	3.01	3.00
query34	16.48	5.13	4.46
query35	4.48	4.45	4.51
query36	0.65	0.48	0.48
query37	0.09	0.06	0.06
query38	0.05	0.04	0.03
query39	0.04	0.02	0.03
query40	0.16	0.13	0.15
query41	0.08	0.02	0.03
query42	0.04	0.02	0.02
query43	0.04	0.03	0.02
Total cold run time: 105.52 s
Total hot run time: 30.57 s

@morrySnow
Copy link
Contributor

run cloud_p0

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jan 22, 2025
@github-actions GitHub Actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

xzj7019
xzj7019 reviewed Jan 23, 2025
View reviewed changes
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java
@@ -201,6 +201,6 @@ public void computeEqualSet(DataTrait.Builder builder) {

@Override
public void computeFd(DataTrait.Builder builder) {
builder.addFuncDepsDG(child().getLogicalProperties().getTrait());
// don't generate fd
Copy link
Contributor

@xzj7019 xzj7019 Jan 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

add sample case to explain why, in the comments line

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

@xzj7019 xzj7019 xzj7019 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.x dev/3.0.x p0_b reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@feiniaofeiafei @hello-stephen @doris-robot @morrySnow @xzj7019