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)canInferNotNullForMarkSlot method get wrong result if fold constant rule is disabled #49428

Merged
merged 3 commits into from
Mar 26, 2025
Merged

[fix](nereids)canInferNotNullForMarkSlot method get wrong result if fold constant rule is disabled #49428

merged 3 commits into from
Mar 26, 2025
+105 −2

Conversation

starocean999
Copy link
Contributor

canInferNotNullForMarkSlot will get wrong result if the expression can't be fold to boolean literal or null. After this pr, the mark slot can be set as not null only if meets one of the following conditions:

  1. conjunct is fold to True literal in all conditions.
  2. conjunct is fold to Null or False literal in all conditions.

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 Mar 24, 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?

@starocean999
Copy link
Contributor Author

run buildall

@starocean999
Copy link
Contributor Author

run buildall

1 similar comment
@starocean999
Copy link
Contributor Author

run buildall

@starocean999
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	25652	5047	5004	5004
q2	2055	286	168	168
q3	10406	1240	676	676
q4	10221	1011	530	530
q5	7553	2354	2357	2354
q6	188	166	135	135
q7	894	741	600	600
q8	9326	1289	1082	1082
q9	6838	5072	5114	5072
q10	6853	2293	1890	1890
q11	487	272	254	254
q12	341	341	213	213
q13	17764	3657	3047	3047
q14	230	223	206	206
q15	526	488	488	488
q16	622	610	583	583
q17	573	853	335	335
q18	7455	7296	7202	7202
q19	2136	941	553	553
q20	310	321	197	197
q21	3845	2673	2463	2463
q22	1040	1023	985	985
Total cold run time: 115315 ms
Total hot run time: 34037 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5193	5084	5071	5071
q2	237	324	227	227
q3	2181	2649	2283	2283
q4	1386	1787	1371	1371
q5	4497	4394	4418	4394
q6	216	165	129	129
q7	2012	1943	1776	1776
q8	2589	2656	2478	2478
q9	7388	7226	7175	7175
q10	2975	3148	2706	2706
q11	560	519	483	483
q12	683	789	623	623
q13	3509	3863	3536	3536
q14	303	298	272	272
q15	544	507	488	488
q16	673	689	646	646
q17	1205	1539	1429	1429
q18	8048	8007	7674	7674
q19	788	773	831	773
q20	1937	2081	1968	1968
q21	5493	4909	4849	4849
q22	1131	1075	1013	1013
Total cold run time: 53548 ms
Total hot run time: 51364 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 193737 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 e782a7bd2875c2aef7c6c57d4b4186fa898ce348, data reload: false

query1	1406	1072	1059	1059
query2	6118	1963	1937	1937
query3	11025	4487	4579	4487
query4	52164	25464	22929	22929
query5	4980	575	480	480
query6	347	196	184	184
query7	4903	497	269	269
query8	331	256	248	248
query9	5520	2611	2614	2611
query10	436	303	249	249
query11	15138	15007	14927	14927
query12	156	112	104	104
query13	1024	507	374	374
query14	10031	6395	6348	6348
query15	204	195	169	169
query16	7110	633	493	493
query17	1047	757	559	559
query18	1579	396	302	302
query19	193	185	164	164
query20	126	123	127	123
query21	214	127	103	103
query22	4486	4721	4266	4266
query23	34318	33393	33621	33393
query24	6658	2420	2447	2420
query25	440	475	429	429
query26	667	281	152	152
query27	2180	548	344	344
query28	3027	2479	2466	2466
query29	550	564	418	418
query30	285	224	195	195
query31	849	897	795	795
query32	72	71	62	62
query33	452	380	309	309
query34	769	844	517	517
query35	806	851	751	751
query36	950	1008	935	935
query37	127	99	78	78
query38	4222	4298	4105	4105
query39	1529	1494	1495	1494
query40	209	128	117	117
query41	59	62	59	59
query42	123	112	109	109
query43	520	520	487	487
query44	1365	847	824	824
query45	195	176	167	167
query46	869	1057	650	650
query47	1913	1923	1818	1818
query48	381	409	312	312
query49	706	533	426	426
query50	707	776	412	412
query51	4281	4363	4298	4298
query52	112	107	97	97
query53	217	252	180	180
query54	499	495	411	411
query55	88	86	91	86
query56	274	265	253	253
query57	1198	1178	1127	1127
query58	251	267	239	239
query59	2742	3012	2723	2723
query60	288	295	253	253
query61	135	127	138	127
query62	775	755	695	695
query63	223	192	178	178
query64	1733	1063	796	796
query65	4551	4523	4379	4379
query66	735	405	312	312
query67	16155	15838	15781	15781
query68	7253	891	513	513
query69	598	308	266	266
query70	1210	1089	1088	1088
query71	499	299	267	267
query72	5739	5071	5017	5017
query73	1436	562	344	344
query74	8923	9100	8785	8785
query75	4048	3253	2714	2714
query76	4207	1200	751	751
query77	700	369	279	279
query78	10032	10072	9317	9317
query79	4073	822	559	559
query80	677	529	437	437
query81	501	252	216	216
query82	484	128	98	98
query83	234	175	155	155
query84	282	102	74	74
query85	867	357	309	309
query86	362	299	301	299
query87	4424	4532	4476	4476
query88	3623	2255	2256	2255
query89	441	309	276	276
query90	1979	217	219	217
query91	141	140	112	112
query92	83	57	55	55
query93	2394	1059	635	635
query94	672	407	313	313
query95	349	280	273	273
query96	487	566	276	276
query97	3360	3432	3321	3321
query98	227	202	200	200
query99	1464	1386	1276	1276
Total cold run time: 299512 ms
Total hot run time: 193737 ms

@doris-robot
Copy link

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

query1	0.04	0.04	0.02
query2	0.11	0.11	0.11
query3	0.25	0.19	0.19
query4	1.59	0.19	0.20
query5	0.59	0.57	0.57
query6	1.18	0.72	0.72
query7	0.02	0.02	0.01
query8	0.05	0.04	0.03
query9	0.57	0.53	0.54
query10	0.58	0.60	0.59
query11	0.15	0.11	0.11
query12	0.15	0.11	0.12
query13	0.61	0.60	0.60
query14	2.70	2.68	2.81
query15	0.93	0.85	0.84
query16	0.38	0.37	0.38
query17	1.01	1.01	1.02
query18	0.20	0.20	0.20
query19	1.94	1.91	1.88
query20	0.01	0.01	0.02
query21	15.40	0.88	0.55
query22	0.74	1.26	0.69
query23	14.90	1.37	0.60
query24	6.86	1.85	0.77
query25	0.49	0.14	0.07
query26	0.62	0.16	0.13
query27	0.05	0.05	0.05
query28	9.82	0.85	0.42
query29	12.54	4.05	3.32
query30	0.25	0.09	0.06
query31	2.81	0.59	0.40
query32	3.22	0.55	0.50
query33	3.11	3.03	3.04
query34	15.72	5.13	4.45
query35	4.52	4.47	4.51
query36	0.66	0.49	0.48
query37	0.09	0.06	0.06
query38	0.05	0.04	0.04
query39	0.03	0.03	0.02
query40	0.18	0.13	0.12
query41	0.08	0.03	0.02
query42	0.03	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 105.27 s
Total hot run time: 31.07 s

@starocean999 starocean999 marked this pull request as ready for review March 25, 2025 03:43
@morrySnow morrySnow added the p0_w label Mar 25, 2025
@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Mar 25, 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.

@starocean999 starocean999 merged commit 7b6d2e2 into apache:master Mar 26, 2025
27 of 29 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 26, 2025
@starocean999
[fix](nereids)canInferNotNullForMarkSlot method get wrong result if f…
39284d3 
…old constant rule is disabled (#49428)

canInferNotNullForMarkSlot will get wrong result if the expression can't
be fold to boolean literal or null. After this pr, the mark slot can be
set as not null only if meets one of the following conditions:
1. conjunct is fold to True literal in all conditions.
2. conjunct is fold to Null or False literal in all conditions.
@github-actions github-actions bot mentioned this pull request Mar 26, 2025
Closed
github-actions bot pushed a commit that referenced this pull request Mar 26, 2025
@starocean999
[fix](nereids)canInferNotNullForMarkSlot method get wrong result if f…
64287e7 
…old constant rule is disabled (#49428)

canInferNotNullForMarkSlot will get wrong result if the expression can't
be fold to boolean literal or null. After this pr, the mark slot can be
set as not null only if meets one of the following conditions:
1. conjunct is fold to True literal in all conditions.
2. conjunct is fold to Null or False literal in all conditions.
@github-actions github-actions bot mentioned this pull request Mar 26, 2025
Closed
This was referenced Mar 28, 2025
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@englefly englefly englefly 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.10-merged dev/3.0.x p0_w reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@starocean999 @Thearas @doris-robot @englefly @morrySnow @yiguolei