Skip to content

[fix](fd) Guard uniform aggregate inference by participation - #67881

Open
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix/uniform-aggregate-participation
Open

[fix](fd) Guard uniform aggregate inference by participation#67881
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix/uniform-aggregate-participation

Conversation

@morrySnow

Copy link
Copy Markdown
Contributor

Problem

When an aggregate groups by a unique, non-null key, each group contains one row. The planner used that fact to mark every COUNT and NDV output as uniform. That is not true for nullable arguments: COUNT(v) and NDV(v) return 0 for a null value and 1 for a non-null value. An outer aggregation can consequently remove such an output from its group keys and merge rows that must remain separate.

Root cause

The logical and physical aggregate trait derivations classified an output as uniform solely from the aggregate function class. They did not distinguish COUNT(*) from argument-based aggregates or check whether the complete argument expressions always participate in the aggregate.

Reproduction

Create a unique-key table containing two rows whose nullable value differs:

create table uniform_agg_witness (
    pk int not null,
    b int not null,
    v int null
) unique key(pk)
distributed by hash(pk) buckets 1
properties("replication_num"="1");

insert into uniform_agg_witness values (1, 7, null), (2, 7, 9);

select b, c, count(*) as n, sum(h) as sh
from (
    select pk, b, count(v) as c, ndv(v) as h
    from uniform_agg_witness
    group by pk, b
) s
group by b, c
order by b, c;

The invalid uniform trait removed c from the outer group keys and produced one merged row. The correct result has separate (7, 0) and (7, 1) groups.

Fix

  • Share one uniform-aggregate proof between logical and physical aggregate plans.
  • Keep COUNT(*) uniform for a single-row group.
  • Treat argument-based COUNT and NDV as uniform only when every complete argument expression is definitely non-null.
  • Default all other cases to non-uniform. This conservatively rejects nullable arguments, nullable conditional expressions, narrowing and try casts whose result may be null, multi-argument counts with any nullable argument, and null-extended outer-join outputs.
  • Preserve the safe optimization for non-null arguments.

Tests

  • ./run-fe-ut.sh --run org.apache.doris.nereids.properties.UniformTest (12 tests passed)
  • ./build.sh --fe
  • ./run-regression-test.sh --run -f regression-test/suites/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.groovy ... -forceGenOut
  • Re-ran the same regression suite normally against the generated expected output.

The regression asserts both results and plan group keys: nullable COUNT/NDV remain in the outer grouping, while non-null COUNT still permits safe group-key elimination.

@morrySnow

Copy link
Copy Markdown
Contributor Author

run buildall

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

### What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: COUNT and NDV outputs were inferred as uniform for single-row groups solely from aggregate shape. When an argument is nullable, row participation can differ by group, so downstream GROUP BY elimination can merge distinct results and change query output. Share a conservative participation proof between logical and physical aggregates: COUNT(*) is safe, while argument-based COUNT and NDV require every complete argument expression to be definitely non-null.

### Release note

Prevent unsafe GROUP BY elimination for aggregates whose row participation can vary.

### Check List (For Author)

- Test: Unit tests, regression test, and FE build
    - `UniformTest` (12 tests passed)
    - `eliminate_group_by_key_by_uniform` regression suite
    - `DISABLE_BUILD_UI=ON ./build.sh --fe`
- Behavior changed: Yes. Uniform traits are no longer inferred when nullable aggregate arguments can change participation.
- Does this need documentation: No
@morrySnow
morrySnow force-pushed the fix/uniform-aggregate-participation branch from 65b831b to 8b38b29 Compare September 11, 2026 19:21
@morrySnow

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 85.71% (6/7) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17561	3121	3079	3079
q2	2069	253	212	212
q3	10281	1053	511	511
q4	4677	253	205	205
q5	7665	621	376	376
q6	138	115	92	92
q7	520	496	381	381
q8	9237	898	901	898
q9	3419	2412	2348	2348
q10	6494	877	680	680
q11	386	195	173	173
q12	615	259	199	199
q13	18141	1525	1148	1148
q14	153	152	137	137
q15	q16	425	394	363	363
q17	1296	841	834	834
q18	3020	2223	2193	2193
q19	1094	810	756	756
q20	369	285	200	200
q21	4850	1694	1846	1694
q22	325	266	231	231
Total cold run time: 92735 ms
Total hot run time: 16710 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3452	3410	3344	3344
q2	507	405	384	384
q3	2254	2357	2133	2133
q4	1180	1154	877	877
q5	2152	2108	2103	2103
q6	168	121	87	87
q7	1002	933	888	888
q8	1601	1398	1420	1398
q9	3117	3083	3059	3059
q10	1861	1783	1617	1617
q11	359	269	249	249
q12	451	429	338	338
q13	1466	1555	1171	1171
q14	165	180	167	167
q15	q16	404	394	359	359
q17	3540	3341	3195	3195
q18	4862	4438	4700	4438
q19	860	796	781	781
q20	1163	1003	872	872
q21	3879	3108	3343	3108
q22	393	345	316	316
Total cold run time: 34836 ms
Total hot run time: 30884 ms

@hello-stephen

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

query5	4265	416	336	336
query6	388	131	124	124
query7	4953	430	227	227
query8	302	124	120	120
query9	8729	2873	2854	2854
query10	398	217	176	176
query11	5375	1048	929	929
query12	131	74	66	66
query13	1190	450	312	312
query14	6128	2182	2077	2077
query14_1	1981	1964	1953	1953
query15	178	119	110	110
query16	943	371	358	358
query17	794	452	367	367
query18	2328	329	238	238
query19	167	139	111	111
query20	70	70	70	70
query21	209	99	88	88
query22	5516	5329	5361	5329
query23	6948	6221	5977	5977
query23_1	6007	6101	6130	6101
query24	7349	1078	747	747
query24_1	765	748	776	748
query25	440	304	278	278
query26	1222	219	129	129
query27	2804	445	258	258
query28	4634	1479	1486	1479
query29	956	427	357	357
query30	258	156	132	132
query31	828	405	329	329
query32	140	74	76	74
query33	454	206	188	188
query34	976	816	471	471
query35	402	391	347	347
query36	560	589	533	533
query37	123	79	69	69
query38	1016	846	820	820
query39	468	504	486	486
query39_1	455	448	473	448
query40	214	92	79	79
query41	60	57	56	56
query42	75	76	74	74
query43	244	242	211	211
query44	995	532	535	532
query45	112	114	102	102
query46	777	854	502	502
query47	763	751	698	698
query48	327	316	235	235
query49	556	265	186	186
query50	728	264	183	183
query51	8061	7898	8032	7898
query52	65	69	59	59
query53	193	201	152	152
query54	196	166	139	139
query55	75	56	54	54
query56	198	178	173	173
query57	662	654	660	654
query58	326	178	160	160
query59	1218	1240	1090	1090
query60	243	173	168	168
query61	110	101	110	101
query62	370	212	187	187
query63	179	137	135	135
query64	2747	765	651	651
query65	1606	1582	1611	1582
query66	1914	258	209	209
query67	11011	9579	9656	9579
query68	2765	1224	681	681
query69	339	230	203	203
query70	668	607	607	607
query71	243	172	168	168
query72	2437	1658	1438	1438
query73	669	586	314	314
query74	1555	1215	1110	1110
query75	1159	1096	956	956
query76	2286	715	509	509
query77	253	259	212	212
query78	3956	3656	3223	3223
query79	2490	856	592	592
query80	1574	319	266	266
query81	494	153	136	136
query82	745	122	99	99
query83	274	202	188	188
query84	303	109	87	87
query85	797	340	270	270
query86	391	168	175	168
query87	1017	973	894	894
query88	2757	2094	2097	2094
query89	286	199	171	171
query90	1986	134	133	133
query91	129	119	98	98
query92	79	63	68	63
query93	1451	1118	672	672
query94	629	240	211	211
query95	535	316	224	224
query96	781	598	255	255
query97	1026	1043	1000	1000
query98	159	135	132	132
query99	420	344	311	311
Total cold run time: 178946 ms
Total hot run time: 81336 ms

@hello-stephen

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

query1	0.01	0.00	0.00
query2	0.07	0.04	0.04
query3	0.25	0.11	0.10
query4	1.60	0.10	0.09
query5	0.17	0.16	0.16
query6	1.27	0.67	0.68
query7	0.03	0.01	0.00
query8	0.04	0.03	0.03
query9	0.28	0.21	0.22
query10	0.35	0.35	0.34
query11	0.17	0.11	0.12
query12	0.15	0.12	0.12
query13	0.30	0.30	0.30
query14	0.45	0.44	0.43
query15	0.36	0.37	0.35
query16	0.21	0.25	0.22
query17	0.70	0.67	0.65
query18	0.18	0.17	0.16
query19	1.16	1.15	1.17
query20	0.01	0.00	0.00
query21	15.45	0.15	0.11
query22	5.09	0.05	0.04
query23	16.19	0.26	0.11
query24	3.05	0.34	0.25
query25	0.12	0.03	0.03
query26	0.75	0.17	0.11
query27	0.04	0.03	0.02
query28	3.66	0.57	0.26
query29	12.44	3.15	2.58
query30	0.27	0.11	0.12
query31	2.76	0.37	0.17
query32	3.51	0.32	0.24
query33	1.50	1.47	1.56
query34	15.34	2.13	1.79
query35	1.74	1.72	1.74
query36	0.45	0.29	0.29
query37	0.06	0.04	0.03
query38	0.05	0.03	0.03
query39	0.03	0.03	0.02
query40	0.11	0.08	0.08
query41	0.08	0.02	0.03
query42	0.03	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 90.52 s
Total hot run time: 14.66 s

@morrySnow morrySnow changed the title [fix](nereids) Guard uniform aggregate inference by participation [fix](fd) Guard uniform aggregate inference by participation Sep 12, 2026
hello-stephen pushed a commit that referenced this pull request Sep 12, 2026
…67897)

Since today every `Doris_DorisCloudRegression_VaultP0` run dies in the
`run` step before executing a single test, e.g.
#67883 (TeamCity build 39010) and
#67881 / #67882 / #67885 / #67886 / #67892 / #67893:

```
doris-external--minio Pulling
doris-external--minio Error
Error response from daemon: pull access denied for minio/minio, repository does not exist or may require 'docker login': denied: requested access to the resource is denied
ERROR: start minio docker twice failed
```

MinIO stopped publishing container images in October 2025 (the project
is a source-only distribution now, see minio/minio#21647) and the
`minio/minio` and `minio/mc` repositories have since been removed from
Docker Hub altogether
(`https://hub.docker.com/v2/repositories/minio/minio/` answers 404, same
for `minio/mc`). The few VaultP0 runs that still pass do so only on
agents that have the image cached locally (their logs have no `Pulling`
line). The iceberg, hudi and polaris third-party fixtures,
`test_file_cache_warmup_read_metrics_docker` (which runs a `docker run
minio/minio` itself), the all-in-one `cloud.yml` and the datalake
samples reference the same images and are one cache eviction away from
the same failure.

`quay.io/minio/minio` and `quay.io/minio/mc` still serve every tag we
use -- `RELEASE.2024-11-07T00-52-20Z`, `RELEASE.2025-01-20T14-49-07Z`,
mc `RELEASE.2025-01-17T23-25-50Z`, the two 2022 tags of the samples and
`latest` -- and MinIO keeps pushing hotfix tags there (latest one dated
2026-04). `docker manifest inspect` resolves all of them (amd64 / arm64
/ ppc64le). So every reference gets the `quay.io/` prefix and the tags
stay exactly as they were: same builds, different registry. The CI
agents already pull from quay.io for the OceanBase fixture.

A longer-term option is to mirror these three tags into the project's
own `doristhirdpartydocker` namespace, which already hosts hive /
zookeeper / kafka / trinodb; that needs someone with push access to that
Docker Hub organization and can follow separately.
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