Repository navigation
[fix](rbo) Preserve union constant rows across rewrites - #67790
Merged
Merged
Conversation
morrySnow
requested review from
924060929,
englefly and
starocean999
as code owners
September 10, 2026 08:40
Contributor
Author
|
run buildall |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
morrySnow
changed the title
Contributor
TPC-H: Total hot run time: 16689 ms |
Contributor
TPC-DS: Total hot run time: 81351 ms |
Contributor
ClickBench: Total hot run time: 14.59 s |
924060929
reviewed
Sep 12, 2026
924060929
left a comment
924060929
left a comment
Contributor
There was a problem hiding this comment.
The rewrite fix itself looks correct. I found one test-framework issue that should be addressed.
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: A UNION ALL could lose existing constant rows when a later rewrite exposed another one-row relation. MergeOneRowRelationIntoUnion rebuilt the union constant-expression list only from current children, so constant rows already stored on the union were omitted. Preserve the existing constant-expression list before appending newly merged rows. Unit tests cover output mapping across rewrites, and named regression queries with generated golden baselines cover GROUP BY, DISTINCT, reverse-order, and multiple-constant-row cases.
### Release note
Fix UNION ALL rewrites to preserve existing constant rows when merging one-row relations.
### Check List (For Author)
- Test: Unit Test and Regression Test
- SetOperationOutputMappingTest
- merge_one_row_relation_into_union
- Behavior changed: Yes. UNION ALL rewrites no longer drop pre-existing constant rows.
- Does this need documentation: No
morrySnow
force-pushed
the
fix/preserve-union-constants
branch
from
September 14, 2026 03:22
ab49394 to
a905b43
Compare
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review conclusion: no new valuable findings. The change is suitable for a comment-only positive review.
The one-line fix preserves the disjoint UNION representation across rewrite-stage re-entry. A production-reachable tree can have an existing metadata row plus a newly exposed one-row child:
Union ALL outputs=[c#U], constant rows=[[1#A]]
OneRowRelation[2 AS c#B]
MergeOneRowRelationIntoUnion now retains [1#A], maps the live child through regularChildOutput, appends [2#B] once, and removes that child. A repeated pass is inapplicable (or returns the original node when only a duplicate-use volatile child remains), so rows are neither lost nor doubled.
Critical checkpoint conclusions:
- Goal and proof: the change directly fixes loss of pre-existing UNION constant rows. The focused unit test fails under the prior behavior and checks identity, duplicate-row multiplicity, child removal, and immediate idempotence. The ordered golden regression cases cover GROUP BY and DISTINCT exposure, reversed branches, and three equal UNION ALL rows.
- Scope and clarity: the implementation is the minimum state-preservation change and uses the existing immutable constant-row representation.
- Parallel paths and stage order: checked whole-plan and CTE rewrites, repeated
PUSH_DOWN_FILTERS,MergeSetOperations, group-by elimination, filter/project-through-union, column pruning, nullable adjustment, and DISTINCT lowering. Every source row remains represented exactly once. - Output correctness: existing constant rows already follow the union output by position and intentionally keep row-owned ExprIds; only live children need remapping. Type/nullability handling, logical-property recomputation, statistics, logical-to-physical conversion, and physical translation remain consistent.
- Qualifiers and special conditions: UNION ALL preserves duplicates. UNION DISTINCT is lowered to an aggregate over the same union changed to ALL, so retaining every input before aggregate deduplication is correct. Volatile-expression evaluation count and rewrite convergence are unchanged.
- Concurrency and lifecycle: no shared mutable state, locking, thread, asynchronous lifecycle, static initialization, or resource-ownership behavior is introduced.
- Configuration, compatibility, persistence, and writes: no configuration, FE-BE field, storage format, rolling-upgrade contract, transaction, persistence, or data-write path changes apply.
- Error handling and observability: no error boundary or invariant handling is weakened, and the narrow planner-state fix does not require new logging or metrics.
- Performance: copying the existing small metadata-row list is linear only when an eligible one-row child is absorbed; no material hot-path regression was found.
- Other risks: the final changed-file and unresolved-candidate sweep found no remaining correctness, compatibility, performance, or coverage issue.
The existing inline request to use named golden-output regression cases is addressed by the current qt_* cases and .out baseline, so it was not duplicated. There was no additional user-provided review focus.
This was a static review as required by the runner contract; I did not independently run builds or tests. All normal and risk-focused review agents converged on NO_NEW_VALUABLE_FINDINGS in round 1. Live submission gate: base df36e174b99557a004bc3ad57faa019da4a7d91f, head a905b4372042c2c2ce8b15f6d0e0f21849b0df0c.
Contributor
TPC-H: Total hot run time: 16916 ms |
Contributor
TPC-DS: Total hot run time: 82539 ms |
Contributor
ClickBench: Total hot run time: 14.74 s |
924060929
approved these changes
Sep 14, 2026
924060929
left a comment
924060929
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewed at a905b43. The previous regression-test concern is addressed: the deterministic cases now use named ordered qt_* queries with a generated .out baseline. The production change correctly preserves existing UNION constant rows before appending newly rewritten one-row children, and the focused unit/regression coverage checks row preservation, duplicate multiplicity, order, and rewrite idempotence. The current cloud_p0 deployment failure (missing libjvm.so/Maven) and NonConcurrent BE LSan/muted routine-load failure are unrelated to this FE planner change.
github-actions Bot
pushed a commit
that referenced
this pull request
Sep 18, 2026
## Problem A UNION ALL could lose constant rows when a later rewrite exposed another one-row child. Queries that combine an existing constant branch with a single-row GROUP BY or DISTINCT branch returned only the newly exposed row. ## Root cause MergeOneRowRelationIntoUnion rebuilt the union constant-expression list from only the current children. If an earlier rewrite pass had already moved a constant child into the union metadata, a later pass replaced that metadata instead of retaining it. ## Reproduction ```sql SELECT 1 AS c UNION ALL SELECT c FROM (SELECT 2 AS c) s GROUP BY c; ``` The result incorrectly omitted the row containing 1. The same behavior occurred with DISTINCT and with the branches reversed. ## Fix Seed the rebuilt constant-expression list with the union existing constant rows before appending newly converted one-row children. This preserves row multiplicity across repeated rewrite passes. Add a focused rewrite unit test and named regression queries with runner-generated golden output for GROUP BY, DISTINCT, reversed branch order, and duplicate constant rows. ## Tests - `SetOperationOutputMappingTest`: 4 tests passed - `merge_one_row_relation_into_union`: named golden regression passed in both force-generation and normal comparison modes - Full FE build with UI disabled: passed - Checkstyle: passed
yiguolei
pushed a commit
that referenced
this pull request
Sep 19, 2026
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
A UNION ALL could lose constant rows when a later rewrite exposed another one-row child. Queries that combine an existing constant branch with a single-row GROUP BY or DISTINCT branch returned only the newly exposed row.
Root cause
MergeOneRowRelationIntoUnion rebuilt the union constant-expression list from only the current children. If an earlier rewrite pass had already moved a constant child into the union metadata, a later pass replaced that metadata instead of retaining it.
Reproduction
The result incorrectly omitted the row containing 1. The same behavior occurred with DISTINCT and with the branches reversed.
Fix
Seed the rebuilt constant-expression list with the union existing constant rows before appending newly converted one-row children. This preserves row multiplicity across repeated rewrite passes.
Add a focused rewrite unit test and named regression queries with runner-generated golden output for GROUP BY, DISTINCT, reversed branch order, and duplicate constant rows.
Tests
SetOperationOutputMappingTest: 4 tests passedmerge_one_row_relation_into_union: named golden regression passed in both force-generation and normal comparison modes