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 months_add/ months_sub/ years_add/years_sub compute wrong result because SimplifyArithmeticComparisonRule #44725

Merged
merged 7 commits into from
Nov 28, 2024
Merged

[fix](nereids) fix months_add/ months_sub/ years_add/years_sub compute wrong result because SimplifyArithmeticComparisonRule #44725

merged 7 commits into from
Nov 28, 2024

Conversation

924060929
Copy link
Contributor

What problem does this PR solve?

fix months_add/ months_sub/ years_add/years_sub compute wrong result because SimplifyArithmeticComparisonRule
introduced by #25180

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

@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.

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?

@924060929
Copy link
Contributor Author

run buildall

morrySnow
morrySnow previously approved these changes Nov 28, 2024
View reviewed changes
@github-actions GitHub Actions
Copy link
Contributor

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 Nov 28, 2024
@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Nov 28, 2024
@924060929
Copy link
Contributor Author

run buildall

@924060929
Copy link
Contributor Author

run buildall

@924060929
Copy link
Contributor Author

run buildall

yujun777
yujun777 approved these changes Nov 28, 2024
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 previously approved these changes Nov 28, 2024
View reviewed changes
@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 28, 2024
@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Nov 28, 2024
@924060929
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	16241	7412	7242	7242
q2	1119	184	170	170
q3	1440	1203	1241	1203
q4	10258	792	697	697
q5	7620	2724	2612	2612
q6	248	156	152	152
q7	992	633	617	617
q8	9247	1893	1920	1893
q9	6611	6378	6373	6373
q10	6987	2290	2316	2290
q11	459	265	263	263
q12	409	224	220	220
q13	17779	3052	3039	3039
q14	245	207	205	205
q15	564	518	509	509
q16	651	618	593	593
q17	1001	704	573	573
q18	7468	7111	6643	6643
q19	1327	1025	1133	1025
q20	490	192	185	185
q21	3931	3213	3255	3213
q22	377	310	325	310
Total cold run time: 95464 ms
Total hot run time: 40027 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7251	7341	7240	7240
q2	324	233	228	228
q3	2871	2786	2758	2758
q4	1948	1732	1686	1686
q5	5385	5399	5457	5399
q6	219	140	139	139
q7	2086	1717	1715	1715
q8	3274	3396	3439	3396
q9	8583	8529	8535	8529
q10	3507	3443	3412	3412
q11	584	493	511	493
q12	794	566	626	566
q13	4356	3073	3039	3039
q14	286	261	275	261
q15	557	516	514	514
q16	682	618	650	618
q17	1800	1599	1577	1577
q18	7839	7551	7472	7472
q19	1667	1452	1529	1452
q20	2059	1800	1809	1800
q21	5401	5296	5215	5215
q22	627	558	571	558
Total cold run time: 62100 ms
Total hot run time: 58067 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 191415 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 9e82b4a85b5936cb1f4adacf6a30ecd0ee91a34f, data reload: false

query1	971	394	378	378
query2	6517	2074	2059	2059
query3	6700	214	207	207
query4	34045	23450	23398	23398
query5	4316	503	475	475
query6	305	190	197	190
query7	4630	296	301	296
query8	293	236	236	236
query9	9652	2727	2736	2727
query10	485	269	252	252
query11	18434	15291	15226	15226
query12	154	105	109	105
query13	1663	430	430	430
query14	9798	6778	7587	6778
query15	263	196	177	177
query16	8188	443	454	443
query17	1825	590	579	579
query18	2144	304	297	297
query19	357	164	156	156
query20	156	123	117	117
query21	208	99	107	99
query22	4590	4315	4353	4315
query23	34965	34652	34279	34279
query24	11433	2423	2426	2423
query25	689	383	381	381
query26	1833	152	158	152
query27	2769	279	280	279
query28	8198	2451	2431	2431
query29	1049	404	409	404
query30	299	150	146	146
query31	1017	806	823	806
query32	98	57	56	56
query33	792	295	284	284
query34	943	516	534	516
query35	890	746	736	736
query36	1107	966	939	939
query37	191	76	76	76
query38	4423	4317	4327	4317
query39	1473	1429	1417	1417
query40	276	99	100	99
query41	46	44	44	44
query42	113	96	100	96
query43	525	490	516	490
query44	1264	812	823	812
query45	191	167	169	167
query46	1112	702	712	702
query47	1940	1832	1860	1832
query48	406	316	326	316
query49	1283	399	383	383
query50	808	398	403	398
query51	7287	7158	7045	7045
query52	103	94	94	94
query53	267	186	182	182
query54	1174	403	414	403
query55	78	79	77	77
query56	255	241	249	241
query57	1320	1172	1181	1172
query58	242	225	218	218
query59	3202	2923	3006	2923
query60	269	249	250	249
query61	121	113	134	113
query62	864	676	696	676
query63	218	190	190	190
query64	5030	712	645	645
query65	3263	3205	3253	3205
query66	1437	316	334	316
query67	15996	15763	15548	15548
query68	5517	540	557	540
query69	415	249	267	249
query70	1155	1128	1107	1107
query71	390	252	251	251
query72	6395	4062	4017	4017
query73	761	369	368	368
query74	10508	9129	9060	9060
query75	3478	2654	2682	2654
query76	3327	1072	1066	1066
query77	538	272	286	272
query78	10354	9429	9629	9429
query79	2369	601	611	601
query80	1141	499	451	451
query81	571	228	231	228
query82	916	117	118	117
query83	248	159	148	148
query84	246	72	79	72
query85	1394	308	296	296
query86	434	308	303	303
query87	4877	4664	4538	4538
query88	4047	2240	2285	2240
query89	416	309	294	294
query90	2190	190	191	190
query91	142	106	104	104
query92	71	51	51	51
query93	1684	541	532	532
query94	1143	299	302	299
query95	358	257	246	246
query96	617	288	281	281
query97	2876	2690	2735	2690
query98	225	198	195	195
query99	1539	1308	1321	1308
Total cold run time: 307295 ms
Total hot run time: 191415 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.04
query2	0.08	0.04	0.04
query3	0.23	0.08	0.07
query4	1.60	0.10	0.11
query5	0.43	0.40	0.43
query6	1.16	0.66	0.67
query7	0.02	0.02	0.02
query8	0.04	0.03	0.03
query9	0.56	0.53	0.51
query10	0.55	0.57	0.55
query11	0.14	0.11	0.10
query12	0.14	0.11	0.12
query13	0.62	0.61	0.60
query14	2.82	2.83	2.81
query15	0.92	0.82	0.82
query16	0.40	0.39	0.40
query17	1.07	1.07	1.06
query18	0.23	0.21	0.21
query19	1.94	1.92	1.97
query20	0.01	0.01	0.01
query21	15.37	0.58	0.55
query22	2.68	1.73	1.90
query23	17.07	0.94	0.75
query24	2.90	1.96	1.15
query25	0.29	0.17	0.06
query26	0.65	0.13	0.15
query27	0.05	0.05	0.04
query28	9.92	1.09	1.07
query29	12.59	3.27	3.24
query30	0.25	0.06	0.07
query31	2.87	0.41	0.39
query32	3.24	0.45	0.48
query33	3.00	2.99	3.00
query34	17.07	4.47	4.44
query35	4.53	4.44	4.52
query36	0.66	0.48	0.48
query37	0.09	0.07	0.06
query38	0.05	0.04	0.04
query39	0.03	0.02	0.02
query40	0.17	0.12	0.12
query41	0.07	0.03	0.02
query42	0.03	0.02	0.02
query43	0.03	0.04	0.03
Total cold run time: 106.61 s
Total hot run time: 32.72 s

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

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

@924060929 924060929 merged commit 1ae91b8 into apache:master Nov 28, 2024
23 of 24 checks passed
924060929 added a commit to 924060929/incubator-doris that referenced this pull request Nov 30, 2024
@924060929
[fix](nereids) fix months_add/ months_sub/ years_add/years_sub comput…
28f1362 
…e wrong result because SimplifyArithmeticComparisonRule (apache#44725)

fix months_add/ months_sub/ years_add/years_sub compute wrong result
because SimplifyArithmeticComparisonRule
introduced by apache#25180

(cherry picked from commit 1ae91b8)
@924060929 924060929 mentioned this pull request Nov 30, 2024
Merged
924060929 added a commit to 924060929/incubator-doris that referenced this pull request Nov 30, 2024
@924060929
[fix](nereids) fix months_add/ months_sub/ years_add/years_sub comput…
9c8a4a4 
…e wrong result because SimplifyArithmeticComparisonRule (apache#44725)

fix months_add/ months_sub/ years_add/years_sub compute wrong result
because SimplifyArithmeticComparisonRule
introduced by apache#25180

(cherry picked from commit 1ae91b8)
@924060929 924060929 mentioned this pull request Nov 30, 2024
Merged
@924060929 924060929 added usercase Important user case type label and removed dev/3.0.x-conflict dev/2.1.x-conflict labels Nov 30, 2024
morrySnow pushed a commit that referenced this pull request Dec 2, 2024
@924060929
[fix](nereids) fix months_add/ months_sub/ years_add/years_sub comput…
6b74db7 
…e wrong result because SimplifyArithmeticComparisonRule (#44725) (#44812)

cherry pick from #44725
morrySnow pushed a commit that referenced this pull request Dec 2, 2024
@924060929
[fix](nereids) fix months_add/ months_sub/ years_add/years_sub comput…
7337930 
…e wrong result because SimplifyArithmeticComparisonRule (#44725) (#44813)

cherry pick from #44725
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@morrySnow morrySnow morrySnow approved these changes

@yujun777 yujun777 yujun777 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.4-merged p0_w reviewed usercase Important user case type label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@924060929 @doris-robot @yujun777 @morrySnow