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

[feat](nereids) add rewrite rule :EliminateGroupByKeyByUniform #43391

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

[feat](nereids) add rewrite rule :EliminateGroupByKeyByUniform #43391

wants to merge 12 commits into from

Conversation

feiniaofeiafei
Copy link
Contributor

@feiniaofeiafei feiniaofeiafei commented Nov 7, 2024 •
edited
Loading

What problem does this PR solve?

This PR introduces two main changes:

  1. Adds an optional constant value to the uniform attribute in DataTrait. A slot with a constant value that is not null will be considered uniform and not null.
  2. Introduces a new transform rule: EliminateGroupByKeyByUniform, which utilizes the newly added part of the uniform attribute. Following is example transformation:
    +--aggregate(group by a,b output a,b,max(c))
    (a is uniform and not null: e.g. a is projection 2 as a in logicalProject)
    ->
    +--aggregate(group by b output b,any_value(a) as a,max(c))

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 colde files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

  • Release note

    None

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

morrySnow
morrySnow reviewed Nov 7, 2024
View reviewed changes
...ain/java/org/apache/doris/nereids/rules/rewrite/PredicateDrivenProjectionSimplification.java Outdated
import java.util.Map;
import java.util.Set;

/**ProjectFilterTransform*/
Copy link
Contributor

Choose a reason for hiding this comment

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

class comment should contain what the rule want to do and how

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17572	7461	7337	7337
q2	2060	177	184	177
q3	10660	1082	1143	1082
q4	10562	879	824	824
q5	7758	3071	3071	3071
q6	247	144	145	144
q7	1022	602	611	602
q8	9373	1971	2024	1971
q9	6583	6458	6555	6458
q10	7115	2460	2429	2429
q11	483	250	262	250
q12	404	210	212	210
q13	17791	2993	3005	2993
q14	241	216	212	212
q15	587	526	517	517
q16	673	581	600	581
q17	979	528	557	528
q18	7272	6753	6650	6650
q19	1338	1025	942	942
q20	467	182	176	176
q21	3968	3302	3246	3246
q22	1119	989	1005	989
Total cold run time: 108274 ms
Total hot run time: 41389 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7336	7257	7293	7257
q2	346	247	258	247
q3	2996	2989	2925	2925
q4	2141	1903	1841	1841
q5	5760	5829	5849	5829
q6	227	143	142	142
q7	2286	1856	1841	1841
q8	3447	3561	3556	3556
q9	8912	8918	8966	8918
q10	3612	3637	3582	3582
q11	649	510	515	510
q12	851	627	650	627
q13	10088	3218	3167	3167
q14	322	270	298	270
q15	601	579	562	562
q16	674	656	648	648
q17	1859	1667	1679	1667
q18	8188	7827	7627	7627
q19	1714	1587	1579	1579
q20	2092	1899	1901	1899
q21	5586	5514	5440	5440
q22	1154	1052	1047	1047
Total cold run time: 70841 ms
Total hot run time: 61181 ms

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17613	7399	7282	7282
q2	2069	186	191	186
q3	10529	1055	1165	1055
q4	10498	905	880	880
q5	7755	3087	3006	3006
q6	239	145	149	145
q7	1028	630	617	617
q8	9380	2020	2087	2020
q9	6559	6410	6417	6410
q10	7101	2406	2413	2406
q11	459	252	253	252
q12	397	212	209	209
q13	17793	3006	3027	3006
q14	237	210	205	205
q15	577	508	506	506
q16	652	570	588	570
q17	966	544	504	504
q18	7337	6755	6698	6698
q19	1332	1066	1014	1014
q20	450	180	179	179
q21	3964	3291	3182	3182
q22	1120	1014	960	960
Total cold run time: 108055 ms
Total hot run time: 41292 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7300	7283	7206	7206
q2	351	257	249	249
q3	2974	2966	2995	2966
q4	2067	1948	1811	1811
q5	5756	5801	5840	5801
q6	232	137	139	137
q7	2300	1846	1850	1846
q8	3418	3524	3531	3524
q9	8936	8919	8888	8888
q10	3611	3561	3559	3559
q11	614	500	498	498
q12	848	651	627	627
q13	10752	3189	3254	3189
q14	297	273	268	268
q15	607	568	548	548
q16	698	641	650	641
q17	1855	1644	1613	1613
q18	8201	7694	7757	7694
q19	1679	1505	1511	1505
q20	2171	1885	1873	1873
q21	5723	5430	5600	5430
q22	1161	1030	1053	1030
Total cold run time: 71551 ms
Total hot run time: 60903 ms

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei feiniaofeiafei force-pushed the project_filter_transform_const branch 2 times, most recently from 6f13f10 to 892b7ec Compare November 13, 2024 04:22
@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei feiniaofeiafei force-pushed the project_filter_transform_const branch 2 times, most recently from 6b4a551 to 852065c Compare November 14, 2024 09:46
@feiniaofeiafei
Copy link
Contributor Author

run buidlall

@feiniaofeiafei feiniaofeiafei force-pushed the project_filter_transform_const branch 2 times, most recently from b393ff3 to 0c470a6 Compare November 14, 2024 10:04
@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei feiniaofeiafei changed the title [feat](nereids) add rewrite rule :EliminateGroupByKeyByUniform and PredicateDrivenProjectionSimplification [feat](nereids) add rewrite rule :EliminateGroupByKeyByUniform Nov 14, 2024
@feiniaofeiafei feiniaofeiafei force-pushed the project_filter_transform_const branch 2 times, most recently from 73c21af to c2bc25a Compare November 15, 2024 10:39
@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei feiniaofeiafei force-pushed the project_filter_transform_const branch 2 times, most recently from cdb345c to c99e369 Compare November 15, 2024 10:47
@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei feiniaofeiafei force-pushed the project_filter_transform_const branch 2 times, most recently from 1d2ad46 to d211163 Compare November 15, 2024 12:01
@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei
Copy link
Contributor Author

run buildall

1 similar comment
@feiniaofeiafei
Copy link
Contributor Author

run buildall

@feiniaofeiafei
[feat](nereids) add rewrite rule :EliminateGroupByKeyByUniform and Pr…
89a5890 
…edicateDrivenProjectionSimplification

add license and class describe

group by is empty then not tranform in EliminateGroupByKeyByUniform

remove transform PredicateDrivenProjectionSimplification
@feiniaofeiafei
enhance DataTrait uniform, add const value to uniform description
b9672da 
fix code style

change getAllUniformAndNotNull in data trait builder

change qualify compute uniform

consider a=null predicate and change isUniformAndNotNull logic

use Optional in Uniform Description

if uniform slot can not get const value, use putIfAbsent when add to slotUniformValue

add logical project constant value to uniform description
@feiniaofeiafei
add test for rule EliminateGroupByKeyByUniform and change computeUnif…
91b43c1 
…orm in LogicalProject
@feiniaofeiafei
add test for rule EliminateGroupByKeyByUniform and change computeUnif…
5a4819c 
…orm in LogicalProject
@feiniaofeiafei
fix logical join uniform properties problem
4e9d980
@feiniaofeiafei
use CustomRewriter reimplement, and rewrite upper exprid to prevent e…
52520cb 
…xprid becoming duplicated
@feiniaofeiafei
fix code style
c9bcf6f
@feiniaofeiafei
not rewrite in cte producer, and change regression mv affected by thi…
48118ed 
…s rule, and add some cases
@feiniaofeiafei
fix code style
b7c2db4
@feiniaofeiafei
fix compile
768f04e
@feiniaofeiafei
comment out some ut, and add regression comment
cd5d8ca
@feiniaofeiafei
fix ut style
7821e01
@feiniaofeiafei
Copy link
Contributor Author

run buildall

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

@morrySnow morrySnow morrySnow left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@feiniaofeiafei @doris-robot @morrySnow