[fix](fd) Guard uniform aggregate inference by participation - #67881
Open
morrySnow wants to merge 1 commit into
Open
[fix](fd) Guard uniform aggregate inference by participation#67881morrySnow wants to merge 1 commit into
morrySnow wants to merge 1 commit into
Conversation
morrySnow
requested review from
924060929,
englefly and
starocean999
as code owners
September 11, 2026 16:58
Contributor
Author
|
run buildall |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
### 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
force-pushed
the
fix/uniform-aggregate-participation
branch
from
September 11, 2026 19:21
65b831b to
8b38b29
Compare
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 16710 ms |
Contributor
TPC-DS: Total hot run time: 81336 ms |
Contributor
ClickBench: Total hot run time: 14.66 s |
8 tasks
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When an aggregate groups by a unique, non-null key, each group contains one row. The planner used that fact to mark every
COUNTandNDVoutput as uniform. That is not true for nullable arguments:COUNT(v)andNDV(v)return0for a null value and1for 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:
The invalid uniform trait removed
cfrom the outer group keys and produced one merged row. The correct result has separate(7, 0)and(7, 1)groups.Fix
COUNT(*)uniform for a single-row group.COUNTandNDVas uniform only when every complete argument expression is definitely non-null.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 ... -forceGenOutThe regression asserts both results and plan group keys: nullable
COUNT/NDVremain in the outer grouping, while non-nullCOUNTstill permits safe group-key elimination.