Skip to content

[fix](nereids) Require safe proofs for aggregate group key elimination - #67884

Open
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix/safe-aggregate-group-key-simplification
Open

[fix](nereids) Require safe proofs for aggregate group key elimination#67884
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix/safe-aggregate-group-key-simplification

Conversation

@morrySnow

@morrySnow morrySnow commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Problem

Aggregate group-key rewrites could remove expressions merely because they referenced the same input slot or were redundant according to child functional dependencies. That can merge distinct groups and can suppress observable errors. For example, a DECIMAL(38,0) key multiplied by 10 must raise arithmetic overflow for the maximum value, but dropping that key made the query succeed.

Root cause

The local simplification rule inferred a determinant from all group expressions instead of requiring one to exist. A second functional-dependency rule compared only input-slot sets, so d and d * 10 appeared equivalent. Neither path proved that a derived expression was total, nor did the latter safely resolve Project/Alias-produced slots. The generic injective-cast predicate is also too broad for character and complex types.

Reproduction

Create a table containing the maximum DECIMAL(38,0) value and run GROUP BY d, d * 10. The multiplication is evaluated by SQL and raises arithmetic overflow. Before this change, group-key elimination could discard d * 10 and suppress the error. Related non-injective decimal expressions could also collapse groups.

Fix

  • Share one fail-closed group-key safety utility across both rewrite rules.
  • Require an actual bare Slot in the group list before local dependent-key removal; never synthesize a determinant.
  • Limit removable derived keys to total integral Add/Subtract/Multiply with one non-null integral literal and non-failing Slot/Cast lineage.
  • Audit cast families and reject string-like, complex, narrowing ordinary casts, Decimal arithmetic, division, floating arithmetic, and unknown cases.
  • Resolve Project/Alias lineage only through a unique definition ending at a catalog relation.
  • Preserve schema functional-dependency elimination and the existing ANY_VALUE/ExprId output rewrite.

Tests

  • 39 focused FE unit tests passed (17 EliminateGroupByKeyTest, 22 SimplifyAggGroupByTest).
  • Full FE build and integrated checkstyle passed.
  • The dedicated regression suite passed forceGenOut and normal runs. It verifies safe integral simplification, missing determinants, multiple determinants, cast-only grouping/output plans, decimal collision preservation, and observable DECIMAL overflow.

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

@morrySnow

Copy link
Copy Markdown
Contributor Author

run buildall

@morrySnow
morrySnow force-pushed the fix/safe-aggregate-group-key-simplification branch from 5b56fe2 to 2f35bfb Compare September 11, 2026 20:59
@morrySnow

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 100.00% (1/1) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17586	3087	3075	3075
q2	2065	253	250	250
q3	10258	875	510	510
q4	4665	253	206	206
q5	7670	564	388	388
q6	137	119	95	95
q7	535	500	391	391
q8	9231	873	901	873
q9	3424	2397	2363	2363
q10	6537	844	706	706
q11	387	200	182	182
q12	618	259	198	198
q13	18132	1518	1148	1148
q14	154	147	141	141
q15	q16	439	396	370	370
q17	1374	924	782	782
q18	3107	2252	2266	2252
q19	1253	916	829	829
q20	366	279	197	197
q21	5609	1565	1858	1565
q22	318	269	232	232
Total cold run time: 93865 ms
Total hot run time: 16753 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3481	3399	3391	3391
q2	493	393	367	367
q3	2170	2280	2119	2119
q4	1181	1171	890	890
q5	2184	2119	2084	2084
q6	173	121	88	88
q7	1006	896	816	816
q8	1589	1407	1403	1403
q9	3105	3081	3055	3055
q10	1847	1825	1654	1654
q11	363	264	248	248
q12	451	435	341	341
q13	1466	1550	1134	1134
q14	175	173	168	168
q15	q16	398	395	359	359
q17	3586	3224	3162	3162
q18	4756	4400	4737	4400
q19	856	820	920	820
q20	1014	961	839	839
q21	3861	3120	3322	3120
q22	404	342	316	316
Total cold run time: 34559 ms
Total hot run time: 30774 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 81680 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 2f35bfb327419139856b27b1c81cc86de46c645f, data reload: false

query5	4250	406	345	345
query6	391	144	142	142
query7	4926	433	230	230
query8	286	136	124	124
query9	8700	2860	2889	2860
query10	397	218	170	170
query11	5396	1024	908	908
query12	118	69	67	67
query13	1188	442	323	323
query14	5976	2166	2046	2046
query14_1	1947	1963	1956	1956
query15	176	118	107	107
query16	930	356	346	346
query17	788	437	372	372
query18	2326	323	257	257
query19	166	138	112	112
query20	70	73	71	71
query21	208	102	88	88
query22	5524	5341	5521	5341
query23	6839	6354	6073	6073
query23_1	6128	6022	6039	6022
query24	7288	1065	767	767
query24_1	758	777	762	762
query25	418	298	260	260
query26	1238	250	132	132
query27	2756	430	247	247
query28	4674	1480	1499	1480
query29	929	432	347	347
query30	246	157	128	128
query31	838	404	324	324
query32	138	79	79	79
query33	467	232	176	176
query34	983	822	480	480
query35	396	402	338	338
query36	585	553	517	517
query37	118	79	70	70
query38	1000	843	823	823
query39	486	483	482	482
query39_1	453	465	445	445
query40	207	90	82	82
query41	58	55	56	55
query42	74	71	73	71
query43	240	249	210	210
query44	993	539	548	539
query45	108	105	99	99
query46	757	818	528	528
query47	738	756	702	702
query48	325	291	240	240
query49	568	243	198	198
query50	758	271	201	201
query51	7920	8063	7998	7998
query52	67	64	56	56
query53	190	200	141	141
query54	196	149	136	136
query55	80	57	54	54
query56	178	161	163	161
query57	692	675	650	650
query58	232	173	160	160
query59	1207	1221	1099	1099
query60	227	183	185	183
query61	121	108	108	108
query62	359	214	187	187
query63	165	140	143	140
query64	2634	713	611	611
query65	1609	1568	1688	1568
query66	1805	263	213	213
query67	9912	9667	9755	9667
query68	3002	1185	731	731
query69	336	224	191	191
query70	668	600	619	600
query71	258	180	165	165
query72	2275	1669	1502	1502
query73	668	616	329	329
query74	1978	1204	1120	1120
query75	1168	1095	952	952
query76	2372	709	523	523
query77	244	257	215	215
query78	4133	3768	3227	3227
query79	2809	864	576	576
query80	1617	330	268	268
query81	512	157	129	129
query82	971	128	95	95
query83	276	216	186	186
query84	306	111	89	89
query85	829	341	279	279
query86	474	176	165	165
query87	1016	962	884	884
query88	2908	2093	2096	2093
query89	281	197	168	168
query90	1999	129	128	128
query91	128	146	98	98
query92	93	72	67	67
query93	2199	1070	714	714
query94	641	254	212	212
query95	511	244	228	228
query96	762	550	249	249
query97	1036	1017	1020	1017
query98	174	131	130	130
query99	417	339	307	307
Total cold run time: 179415 ms
Total hot run time: 81680 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.68 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 2f35bfb327419139856b27b1c81cc86de46c645f, data reload: false

query1	0.01	0.00	0.01
query2	0.07	0.04	0.03
query3	0.25	0.11	0.11
query4	1.60	0.09	0.10
query5	0.17	0.15	0.17
query6	1.28	0.72	0.68
query7	0.03	0.01	0.00
query8	0.04	0.03	0.03
query9	0.30	0.21	0.21
query10	0.34	0.33	0.34
query11	0.16	0.12	0.11
query12	0.15	0.12	0.12
query13	0.31	0.31	0.32
query14	0.46	0.44	0.48
query15	0.36	0.36	0.35
query16	0.24	0.23	0.23
query17	0.71	0.67	0.71
query18	0.17	0.17	0.17
query19	1.19	1.07	1.15
query20	0.02	0.01	0.01
query21	15.48	0.15	0.11
query22	5.07	0.05	0.04
query23	16.18	0.25	0.10
query24	3.01	0.32	0.27
query25	0.10	0.03	0.04
query26	0.82	0.17	0.11
query27	0.03	0.04	0.03
query28	3.67	0.52	0.27
query29	12.50	3.20	2.56
query30	0.26	0.11	0.12
query31	2.77	0.39	0.17
query32	3.52	0.32	0.23
query33	1.37	1.38	1.52
query34	15.37	2.20	1.77
query35	1.75	1.74	1.71
query36	0.54	0.47	0.48
query37	0.06	0.03	0.04
query38	0.05	0.03	0.03
query39	0.04	0.02	0.02
query40	0.12	0.07	0.07
query41	0.08	0.03	0.02
query42	0.03	0.02	0.02
query43	0.04	0.03	0.02
Total cold run time: 90.72 s
Total hot run time: 14.68 s

@morrySnow morrySnow changed the title [fix](nereids) Restrict aggregate group key simplification [fix](rbo) Restrict aggregate group key simplification Sep 12, 2026
@morrySnow
morrySnow marked this pull request as draft September 12, 2026 03:03
### What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: Aggregate group-key rewrites treated expressions that referenced the same slot as interchangeable. One rule could synthesize a missing determinant, while another relied only on child functional dependencies. Non-injective or potentially failing expressions could therefore be removed, merging distinct groups or suppressing evaluation errors. Require an existing bare-slot determinant for local simplification, share fail-closed expression safety checks with functional-dependency elimination, and resolve Project/Alias lineage only when it is complete and unambiguous. Safe integral dependencies and ordinary schema functional dependencies remain optimized, and required output keys keep the existing ANY_VALUE ExprId rewrite.

### Release note

Aggregate group-key elimination now preserves non-injective or potentially failing expressions while retaining proven-safe integral and schema-FD optimizations.

### Check List (For Author)

- Test:
    - Unit Test: EliminateGroupByKeyTest and SimplifyAggGroupByTest (39 tests).
    - Regression test: simplify_agg_group_by force-generated output and normal run.
    - Build: DISABLE_BUILD_UI=ON ./build.sh --fe, including FE checkstyle.
- Behavior changed: Yes. Unsafe group-key elimination is disabled when no existing bare-slot determinant is available, expression evaluation can fail, or lineage cannot be proven; proven-safe elimination remains enabled.
- Does this need documentation: No.
@morrySnow
morrySnow force-pushed the fix/safe-aggregate-group-key-simplification branch from 2f35bfb to ce11ba3 Compare September 12, 2026 11:07
@morrySnow morrySnow changed the title [fix](rbo) Restrict aggregate group key simplification [fix](nereids) Require safe proofs for aggregate group key elimination Sep 12, 2026
@morrySnow

Copy link
Copy Markdown
Contributor Author

run buildall

@morrySnow
morrySnow marked this pull request as ready for review September 12, 2026 11:08
@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17580	3064	3041	3041
q2	2078	266	236	236
q3	10233	880	532	532
q4	4682	262	207	207
q5	7668	577	392	392
q6	140	115	96	96
q7	531	500	388	388
q8	9228	891	956	891
q9	3474	2424	2406	2406
q10	6522	861	697	697
q11	396	195	185	185
q12	608	266	203	203
q13	18121	1532	1175	1175
q14	155	155	139	139
q15	q16	444	404	376	376
q17	1365	919	803	803
q18	3187	2293	2260	2260
q19	1281	911	759	759
q20	373	287	197	197
q21	5648	1655	1926	1655
q22	333	279	237	237
Total cold run time: 94047 ms
Total hot run time: 16875 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3417	3288	3315	3288
q2	505	394	383	383
q3	2230	2330	2225	2225
q4	1219	1189	913	913
q5	2206	2175	2131	2131
q6	170	121	88	88
q7	1039	963	896	896
q8	1612	1397	1403	1397
q9	3174	3153	3155	3153
q10	1902	1827	1646	1646
q11	360	270	250	250
q12	463	434	351	351
q13	1512	1556	1181	1181
q14	173	178	167	167
q15	q16	410	399	362	362
q17	3675	3320	3171	3171
q18	4885	4517	4842	4517
q19	990	871	879	871
q20	1038	972	854	854
q21	3874	3203	3232	3203
q22	400	357	320	320
Total cold run time: 35254 ms
Total hot run time: 31367 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 82827 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 ce11ba3142b1dfbf171b992195922fcd9f4e1d7b, data reload: false

query5	4251	409	345	345
query6	377	132	124	124
query7	4952	415	232	232
query8	283	130	121	121
query9	8701	2905	2915	2905
query10	393	211	194	194
query11	5386	1032	914	914
query12	127	70	71	70
query13	1194	456	320	320
query14	6200	2231	2120	2120
query14_1	2018	1998	1998	1998
query15	179	127	115	115
query16	918	365	351	351
query17	803	442	349	349
query18	2318	321	230	230
query19	153	132	105	105
query20	73	69	77	69
query21	203	107	86	86
query22	5586	5511	5518	5511
query23	7049	6460	6101	6101
query23_1	6254	6165	5926	5926
query24	7252	1111	767	767
query24_1	773	764	782	764
query25	401	281	240	240
query26	1241	229	132	132
query27	2799	429	254	254
query28	4685	1507	1474	1474
query29	906	434	329	329
query30	252	152	129	129
query31	831	393	341	341
query32	128	70	70	70
query33	442	213	173	173
query34	969	872	482	482
query35	378	399	356	356
query36	564	577	526	526
query37	117	92	71	71
query38	1012	858	828	828
query39	517	493	499	493
query39_1	469	478	470	470
query40	208	93	80	80
query41	59	59	56	56
query42	79	77	77	77
query43	247	242	213	213
query44	1007	545	539	539
query45	120	110	105	105
query46	794	832	540	540
query47	784	783	730	730
query48	324	323	239	239
query49	539	266	200	200
query50	788	268	196	196
query51	8110	8194	8173	8173
query52	70	74	76	74
query53	233	204	154	154
query54	228	169	171	169
query55	86	69	55	55
query56	197	187	167	167
query57	697	704	641	641
query58	193	190	181	181
query59	1218	1223	1115	1115
query60	235	180	183	180
query61	109	128	126	126
query62	346	192	176	176
query63	167	144	144	144
query64	2792	758	612	612
query65	1754	1602	1602	1602
query66	1764	263	192	192
query67	10004	9895	9773	9773
query68	2770	1133	734	734
query69	336	227	188	188
query70	671	648	601	601
query71	243	189	175	175
query72	2272	1718	1523	1523
query73	619	547	345	345
query74	1584	1252	1160	1160
query75	1180	1112	971	971
query76	2292	731	528	528
query77	261	247	210	210
query78	4058	3780	3388	3388
query79	2481	829	590	590
query80	1626	338	274	274
query81	492	167	135	135
query82	624	121	96	96
query83	283	211	189	189
query84	294	106	93	93
query85	769	343	284	284
query86	379	180	175	175
query87	1043	994	906	906
query88	2773	2112	2104	2104
query89	288	197	172	172
query90	2013	135	121	121
query91	136	123	110	110
query92	82	67	70	67
query93	1633	1103	700	700
query94	649	274	231	231
query95	531	255	285	255
query96	834	586	279	279
query97	1071	1147	1025	1025
query98	158	136	135	135
query99	420	345	305	305
Total cold run time: 178613 ms
Total hot run time: 82827 ms

@hello-stephen

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

query1	0.01	0.00	0.01
query2	0.08	0.04	0.04
query3	0.25	0.09	0.11
query4	1.61	0.10	0.11
query5	0.18	0.15	0.16
query6	1.26	0.69	0.68
query7	0.03	0.01	0.01
query8	0.04	0.03	0.03
query9	0.29	0.21	0.22
query10	0.35	0.36	0.34
query11	0.17	0.12	0.12
query12	0.15	0.12	0.13
query13	0.31	0.32	0.31
query14	0.47	0.47	0.45
query15	0.37	0.35	0.36
query16	0.23	0.23	0.22
query17	0.68	0.74	0.71
query18	0.18	0.16	0.16
query19	1.28	1.17	1.23
query20	0.01	0.01	0.01
query21	15.47	0.16	0.12
query22	5.11	0.04	0.04
query23	16.16	0.24	0.10
query24	2.97	0.31	0.24
query25	0.10	0.05	0.03
query26	0.77	0.16	0.12
query27	0.04	0.03	0.03
query28	3.62	0.59	0.27
query29	12.43	3.24	2.56
query30	0.25	0.12	0.11
query31	2.76	0.38	0.17
query32	3.53	0.33	0.23
query33	1.41	1.66	1.44
query34	15.40	2.24	1.81
query35	1.79	1.81	1.76
query36	0.46	0.30	0.29
query37	0.06	0.04	0.04
query38	0.04	0.03	0.03
query39	0.03	0.03	0.02
query40	0.12	0.07	0.07
query41	0.07	0.02	0.03
query42	0.03	0.02	0.02
query43	0.03	0.02	0.03
Total cold run time: 90.6 s
Total hot run time: 14.81 s

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