Skip to content

[fix](nereids) abort SimplifyArithmeticComparison rewrite when the ne…#63567

Open
yx-keith wants to merge 1 commit into
apache:masterfrom
yx-keith:fix-arithmetic-comparison-overflow-61761
Open

[fix](nereids) abort SimplifyArithmeticComparison rewrite when the ne…#63567
yx-keith wants to merge 1 commit into
apache:masterfrom
yx-keith:fix-arithmetic-comparison-overflow-61761

Conversation

@yx-keith
Copy link
Copy Markdown
Contributor

…w constant overflows (#61761)

date_sub(i, interval 1 day) <= '9999-12-31' was being rewritten to i <= days_add('9999-12-31', 1). The new constant overflows the DATE domain, but the rule did not detect this: FoldConstantRule.evaluate swallows exceptions in non-debug mode and returns the unfolded expression, so the overflow leaked to runtime as
"Operation day_add of 9999-12-31 00:00:00, 1 out of range".

Eagerly fold the rearranged constant inside tryRearrangeChildren when both sides are literals. A non-Literal result means folding failed, in which case we throw so the outer try/catch falls back to the original comparison. This covers the symmetric overflow case for every operator pair in REARRANGEMENT_MAP (days/weeks/hours/minutes/ seconds + numeric add/sub/divide).

Also add unit tests for the date overflow cases and a regression test based on the original repro.

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

…w constant overflows (apache#61761)

date_sub(i, interval 1 day) <= '9999-12-31' was being rewritten to
i <= days_add('9999-12-31', 1). The new constant overflows the DATE
domain, but the rule did not detect this: FoldConstantRule.evaluate
swallows exceptions in non-debug mode and returns the unfolded
expression, so the overflow leaked to runtime as
"Operation day_add of 9999-12-31 00:00:00, 1 out of range".

Eagerly fold the rearranged constant inside tryRearrangeChildren when
both sides are literals. A non-Literal result means folding failed,
in which case we throw so the outer try/catch falls back to the
original comparison. This covers the symmetric overflow case for
every operator pair in REARRANGEMENT_MAP (days/weeks/hours/minutes/
seconds + numeric add/sub/divide).

Also add unit tests for the date overflow cases and a regression test
based on the original repro.
@hello-stephen
Copy link
Copy Markdown
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?

@yx-keith
Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

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

------ Round 1 ----------------------------------
orders	Doris	NULL	NULL	0	0	0	NULL	0	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL	
============================================
q1	17875	4175	4054	4054
q2	q3	10849	1356	811	811
q4	4691	485	348	348
q5	7590	2243	2041	2041
q6	247	175	137	137
q7	977	789	631	631
q8	9364	1675	1564	1564
q9	6317	5032	4981	4981
q10	6479	2246	1885	1885
q11	439	289	249	249
q12	692	434	290	290
q13	18259	3418	2836	2836
q14	268	265	239	239
q15	q16	824	782	710	710
q17	1037	980	884	884
q18	6990	5799	5581	5581
q19	1263	1243	1077	1077
q20	536	403	263	263
q21	5677	2631	2460	2460
q22	435	358	303	303
Total cold run time: 100809 ms
Total hot run time: 31344 ms

----- Round 2, with runtime_filter_mode=off -----
orders	Doris	NULL	NULL	150000000	42	6422171781	NULL	22778155	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL	
============================================
q1	4427	4345	4293	4293
q2	q3	4545	5067	4418	4418
q4	2154	2248	1416	1416
q5	4474	4315	4923	4315
q6	254	204	153	153
q7	2004	1895	1592	1592
q8	2593	2232	2231	2231
q9	8142	8185	8023	8023
q10	4852	4964	4306	4306
q11	599	466	391	391
q12	750	751	558	558
q13	3296	3740	2941	2941
q14	300	302	281	281
q15	q16	746	732	640	640
q17	1371	1358	1416	1358
q18	7957	7428	6955	6955
q19	1118	1083	1065	1065
q20	2219	2232	1988	1988
q21	5405	4623	4519	4519
q22	550	458	418	418
Total cold run time: 57756 ms
Total hot run time: 51861 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

TPC-DS: Total hot run time: 174911 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 301461a48dbc8b0d366261b40e8246b6168c464c, data reload: false

query5	4342	706	522	522
query6	328	218	194	194
query7	4231	604	313	313
query8	329	235	223	223
query9	8867	4180	4158	4158
query10	458	363	315	315
query11	5823	2588	2338	2338
query12	184	132	132	132
query13	1285	618	443	443
query14	6443	5839	5568	5568
query14_1	4784	4784	4773	4773
query15	227	220	191	191
query16	959	479	443	443
query17	1205	736	619	619
query18	2506	521	371	371
query19	208	203	166	166
query20	140	137	137	137
query21	217	147	125	125
query22	13757	13754	13393	13393
query23	17654	16789	16291	16291
query23_1	16368	16508	16433	16433
query24	7445	1806	1346	1346
query24_1	1348	1349	1324	1324
query25	563	473	420	420
query26	1311	345	177	177
query27	2667	597	345	345
query28	4425	2020	2054	2020
query29	979	648	521	521
query30	315	251	211	211
query31	1154	1100	999	999
query32	85	75	76	75
query33	539	372	295	295
query34	1196	1159	688	688
query35	819	819	718	718
query36	1398	1426	1236	1236
query37	153	109	92	92
query38	3327	3219	3127	3127
query39	935	934	897	897
query39_1	881	896	864	864
query40	237	149	126	126
query41	72	65	68	65
query42	118	118	115	115
query43	353	355	314	314
query44	
query45	234	217	211	211
query46	1125	1217	758	758
query47	2359	2351	2258	2258
query48	420	438	305	305
query49	660	547	415	415
query50	1073	372	261	261
query51	4591	4426	4375	4375
query52	114	113	102	102
query53	267	299	219	219
query54	348	311	276	276
query55	100	96	89	89
query56	338	341	323	323
query57	1492	1418	1312	1312
query58	318	323	279	279
query59	1669	1768	1499	1499
query60	330	333	315	315
query61	157	151	152	151
query62	700	671	592	592
query63	244	202	205	202
query64	2390	824	627	627
query65	
query66	1719	481	355	355
query67	30545	29941	29822	29822
query68	
query69	476	349	313	313
query70	1074	1046	1063	1046
query71	309	269	273	269
query72	3020	2746	2457	2457
query73	816	782	451	451
query74	5170	4996	4816	4816
query75	2734	2661	2303	2303
query76	2270	1177	790	790
query77	422	460	345	345
query78	12894	12798	12150	12150
query79	1516	1080	765	765
query80	656	584	477	477
query81	456	301	255	255
query82	1412	164	131	131
query83	361	288	261	261
query84	266	150	118	118
query85	880	543	458	458
query86	405	346	351	346
query87	3513	3500	3334	3334
query88	3696	2782	2789	2782
query89	456	397	345	345
query90	1866	193	190	190
query91	182	170	137	137
query92	77	81	77	77
query93	1495	1419	893	893
query94	558	344	295	295
query95	676	468	367	367
query96	1060	816	370	370
query97	2764	2750	2641	2641
query98	239	235	233	233
query99	1178	1147	1037	1037
Total cold run time: 257764 ms
Total hot run time: 174911 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 4.11% (3/73) 🎉
Increment coverage report
Complete coverage report

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants