Repository navigation
[fix](fe) Preserve column identity for forwarded USING keys - #69388
Merged
Merged
Conversation
### What problem does this PR solve? Issue Number: N/A Related PR: apache#64718 A qualified reference to a preserved-side USING key can be rejected even when the same key appears unqualified in GROUP BY. For example: ```sql SELECT t1.a, COUNT(*) FROM t1 LEFT JOIN t2 USING(a) GROUP BY a; ``` The USING-key projection creates a fresh Alias for a column forwarded directly from the preserved side. SELECT t1.a then references the original ExprId while GROUP BY a references the new one. Aggregate analysis rejects the query. The same mismatch affects HAVING and ORDER BY. Keep the input ExprId for forwarded USING keys while clearing the qualifier of the unqualified view. Preserve the original qualified outputs and the explicit asterisk output list. FULL OUTER JOIN still builds a new COALESCE expression with its own ExprId, and aggregate validation remains unchanged. ### Release note Fix spurious GROUP BY errors when qualified and unqualified references to a forwarded USING join key are mixed in SELECT, GROUP BY, HAVING, or ORDER BY. ### Check List (For Author) - Test: Unit Test - Complete BindUsingJoinTest suite passed through ./run-fe-ut.sh: 29 tests, 0 failures/errors. - Added complete-planner cases for both reference directions, HAVING/ORDER BY, compound grouping expressions, multiple USING keys, and derived inputs across INNER, LEFT/RIGHT, and SEMI/ANTI joins. - Existing coverage checks star expansion, qualified stars, chained joins, and FULL OUTER COALESCE keys; negative cases keep nullable-side and FULL OUTER key validation intact. - Added an EXPLAIN regression case to test_using_join_merge_key; its SQL is also exercised by the FE full-planner unit test. The regression suite itself has not been run locally. - Checkstyle and git diff --check. - Behavior changed: Yes, valid grouping queries now plan successfully. - Does this need documentation: No.
924060929
requested review from
englefly,
morrySnow and
starocean999
as code owners
October 21, 2026 08:44
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
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.
Request changes for one P1 data-correctness regression: sharing a forwarded USING key's ExprId with its qualified source can silently reorder CTE SELECT-star columns. A producer selecting merged key, payload, qualified key becomes [L,B,L]; LogicalCTEConsumer groups the equal producer keys and exposes [key,key,payload]. The inline finding includes the concrete SQL, reduced plan, expected versus incorrect row, baseline distinction and fix direction. One comment covers both preserved-side branches.
Reviewed all three authoritative changed paths at head 4c48cc1, including parser/binding, scope and slot identity, stars/chained joins/derived inputs, aggregate validation and normalization, CTE construction/inlining, properties, rewrites and physical/result translation. Existing review threads were empty; no existing P0/P1 IDs need carrying forward. No additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and evidence: preserving identity addresses the intended mixed qualified/unqualified GROUP BY/HAVING/ORDER BY rejection, and new identity/full-planner tests target it. The CTE ordering regression prevents accepting the implementation as complete.
- Scope, clarity and reuse: production changes are focused and use the existing withQualifier API; no unrelated refactor.
- Concurrency and locking: statement-local synchronous binding; no new thread, shared mutable state, lock scope or lock-order change.
- Lifecycle and static initialization: immutable plan-owned expressions; no new resource ownership, cleanup or static initialization dependency.
- Configuration: no configuration or session variable added or changed.
- Compatibility: no storage format, function symbol or rolling-upgrade wire contract changed; intended query acceptance expands.
- Thrift/protobuf: no definitions changed; field-ID/enum compatibility checks are inapplicable.
- Parallel paths: preserved left/right inner/outer/semi/anti branches, left-oriented parsed ASOF, downstream mark paths and FULL COALESCE isolation were checked. The finding applies to both forwarded branches; no additional issue found in these paths.
- Special conditions: existing side-selection conditions remain appropriate; preserved-side type/nullability are retained and opposite nullable/FULL identities stay separate. No new defensive or silent-error branch.
- Test coverage: new cases cover seven join forms, both grouping directions, expressions, multiple keys, HAVING/ORDER BY, derived inputs and nullable/FULL negatives. Existing star/chained cases remain. The mixed-key CTE output-order trigger is missing; add a result-checking regression for the inline finding. The new Groovy case only EXPLAINs.
- Expected results: no result files changed; the plain EXPLAIN requires no new result block. The existing nine ordered regression result blocks were inspected against their SQL, including unmatched-row NULLs.
- Observability: existing planner diagnostics suffice for this local identity change; no new operational path requires logs or metrics.
- Transactions, persistence and failover: no EditLog, transaction, version or replay changes.
- Data writes and atomicity: no write/commit implementation changed, but misordered SELECT columns can feed wrong positional values into INSERT SELECT or set operations, as described inline.
- FE/BE variables: none introduced; existing project/result translation and duplicate-aware maps were inspected.
- Performance: standard qualifier copying replaces Alias creation; no added asymptotic work or other substantiated performance issue.
- Other concerns/core invariants: view SQL occurrence indices, lineage/visibility, correlation, grouping-set null extension and ordinary duplicated projections were checked without another verified regression. Read-query data correctness is affected by the P1; visible-version, delete-bitmap and BE memory-accounting paths are untouched, and existing AnalysisException behavior remains.
Converged after 2 review rounds: all three assigned reviewers returned NO_NEW_VALUABLE_FINDINGS on the identical frozen inline set; all candidates and risk items have evidence-backed dispositions, and the final changed-path/anchor sweep is complete. No unresolved static coverage gaps remain. Validation was static only: no builds, tests or query execution were performed, as required by this review contract.
Review converged after 2 round(s).
| } else { | ||
| // LEFT OUTER / INNER / LEFT SEMI / LEFT ANTI / CROSS: use left side | ||
| mergeExpr = new Alias(leftSlot, colName); | ||
| mergeExpr = leftSlot.withQualifier(ImmutableList.of()); |
Contributor
There was a problem hiding this comment.
[P1] Preserve CTE column order when sharing the USING-key ExprId
WITH q(x,y,z) AS (SELECT a,t1.b,t1.a FROM t1 JOIN t2 USING(a)) SELECT * FROM q now exposes its columns in the wrong order. The producer becomes:
Project(a#L, t1.b#B, t1.a#L)
Join(t1.a = t2.a)
AnalyzeCTE wraps this with column names [x#L,y#B,z#L], retaining the ExprIds. LogicalCTEConsumer then keys producerToConsumerOutputMap by producer Slot, whose equality compares only ExprId, and computeOutput() returns that multimap's grouped values(): [x,z,y]. Star expansion binds this order before CTE inlining, which does not repair it. With a matching t1(a,b)=(1,10) row, the result is (1,1,10) instead of (1,10,1); positional consumers such as UNION or INSERT SELECT can consequently receive wrong values. Previously the merged Alias had a separate ExprId, so this query preserved its column order. The right-preserved branch has the same issue.
Preserve producer column positions in the CTE consumer alongside this identity change, and add a result-checking regression with merged and qualified keys separated by a non-key column.
Contributor
Author
There was a problem hiding this comment.
Confirmed locally: the reported SQL fails on the original PR head with expected column order [x,y,z] but actual [x,z,y]. Fixed in 363d34b.
LogicalCTEConsumer now takes its output from the insertion-ordered consumer-to-producer map, whose distinct consumer keys preserve each producer position. Nullability adjustment, producer ExprId replacement and variant subpath pruning also retain that order when rebuilding the map. The USING-key identity fix remains intact.
Added inner/left/right USING CTE order checks with full planning, an actual FE-computed repeated-column result check for (1,10,1), and consumer copy/pruning/nullability/ExprId replacement tests. All 73 focused FE tests passed through run-fe-ut.sh; the full FE build and Checkstyle also passed. The result check uses the FE result-computation path; no BE execution is claimed.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69388 Problem Summary: A CTE selecting a forwarded USING key, a payload column, and the qualified key can expose columns in the wrong order because the reverse producer-to-consumer multimap groups equal producer ExprIds. Use the ordered consumer-to-producer map for output positions and preserve that order when rebuilding mappings for nullability, producer ExprId replacement and variant subpath pruning. Cover inner/left/right USING CTEs, FE-computed result values, consumer copies, pruning and mapping rewrites. ### Release note Preserve CTE result column order when the same source column appears more than once in its output. ### Check List (For Author) - Test: Unit Test - Official run-fe-ut.sh: 73 tests passed across BindUsingJoinTest, LogicalCTEConsumerTest, AnalyzeCTETest and EliminateGroupByKeyTest. - Reproduced the column-order failure before the repair. - Integrated Checkstyle and git diff --check passed. - Behavior changed: Yes, repeated CTE source columns retain their positions. - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Reviewed head 363d34b70578aee08231e8082b6a0151ce199ac0: no new actionable findings. The existing P1 4334911111 is independently confirmed repaired: distinct consumer keys retain [x,y,z] when producer ExprIds repeat, and copies, pruning, nullability/ExprId/type rewrites, variant expansion, inlining and materialized translation preserve explicit column mappings. No existing blocking comment remains applicable from the supplied threads.
All eight changed files and their relevant upstream/downstream paths were covered. No additional user review focus was supplied.
Critical checkpoint conclusions:
- Goal/data correctness: preserved qualified and unqualified USING keys share identity; nullable-side keys remain distinct and FULL keeps a separate COALESCE Alias. Inspected tests address grouping and repeated CTE positions.
- Scope/reuse: focused changes reuse the existing binder, consumer maps and rewrite interfaces; the CTE repair is necessary for the identity change.
- Concurrency/locking: statement-local rewriting and immutable plan-map snapshots; no new threads, shared mutations, catalog IO or locks.
- Lifecycle/static initialization: traced consumer construction through reconstruction, inlining and physical translation; no new resource or static-initialization dependency.
- Configuration: no configuration additions or dynamic-update behavior changes.
- Compatibility: no storage-format, external-symbol or wire-contract change requiring rolling-upgrade handling.
- Thrift/protobuf: no changed definitions.
- Parallel paths: checked left/right inner, outer, semi/anti, FULL, nested USING, qualified/unqualified stars, both CTE inline implementations and materialized mappings. No new rule, stage, mark-join or distribution restriction.
- Conditional checks: preserved-side and FULL branches match their semantics; no speculative fallback or swallowed invariant failure introduced.
- Test coverage: inspected positive/negative grouping, expressions, multiple keys, derived inputs, INNER/LEFT/RIGHT CTE order, consumer copy/prune/nullability/ID tests and FE-computed
(1,10,1)result assertion. - Expected results: no golden result file changed; the new EXPLAIN case exercises planning and existing result cases are deterministic.
- Observability: no new operational path requiring logs/metrics; existing planner diagnostics remain applicable.
- Transactions/persistence/failover: no EditLog, transaction or persisted-metadata changes.
- Writes/atomicity/crash cleanup: no data-write protocol or resource-owning path changed.
- FE/BE propagation: no new transmitted variables or serialization fields; visible-version and delete-bitmap mechanisms are unaffected.
- Performance/memory: query-owned map snapshots retain existing ownership; forwarding removes redundant Alias creation, and the changed entry loops avoid repeated reverse-group traversal. No new obvious hot-path cost found; no benchmark claim.
- Other concerns: final sweep found no unresolved candidate, risk focus or uncovered changed file.
Validation limits: static review only; no builds, style checks or tests were run in this invocation. The author's reported test runs were not independently reproduced. The new aggregate regression checks EXPLAIN/full planning, and the value test computes a constant result in FE; added tests do not establish BE-executed results for materialized USING/variant CTE combinations. Those paths were traced statically with no concrete defect found.
Converged after 1 review round: both complete-coverage reviewers and the separate risk-focused reviewer returned NO_NEW_VALUABLE_FINDINGS; all candidate and risk dispositions were independently checked, and the final sweep is complete.
Review converged after 1 round(s).
924060929
added a commit
to 924060929/incubator-doris
that referenced
this pull request
Oct 21, 2026
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69388 Problem Summary: A CTE selecting a forwarded USING key, a payload column, and the qualified key can expose columns in the wrong order because the reverse producer-to-consumer multimap groups equal producer ExprIds. Use the ordered consumer-to-producer map for output positions and preserve that order when rebuilding mappings for nullability, producer ExprId replacement and variant subpath pruning. Cover inner/left/right USING CTEs, FE-computed result values, consumer copies, pruning and mapping rewrites. Adapt the backported tests to the branch Java 8 API target. ### Release note Preserve CTE result column order when the same source column appears more than once in its output. ### Check List (For Author) - Test: Unit Test - Official run-fe-ut.sh: 73 tests passed across BindUsingJoinTest, LogicalCTEConsumerTest, AnalyzeCTETest and EliminateGroupByKeyTest. - Reproduced the column-order failure before the repair. - Integrated Checkstyle and git diff --check passed. - Behavior changed: Yes, repeated CTE source columns retain their positions. - Does this need documentation: No (cherry picked from commit 363d34b)
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Static review of PR #69388 at 363d34b: no new substantiated findings. All eight changed paths and relevant upstream/downstream behavior were reviewed. The review converged in one round; all three independent reviewers returned NO_NEW_VALUABLE_FINDINGS, and both initial risk focuses have evidence-backed dispositions.
Existing P1 4334911111 is repaired at this head. For producer output [x#L,y#B,z#L], LogicalCTEConsumer now exposes distinct consumer keys in [x,y,z] order. Nullability, producer-ID replacement and variant rebuilding retain forward-map order. Inlining uses that output order; materialization maps producer IDs to all corresponding consumers and constructs projections in consumer order. No existing blocking comment remains applicable from the supplied thread, and none is reposted.
Critical checkpoint conclusions:
- Goal and evidence: preserved-side USING references now share identity for GROUP BY/HAVING/ORDER BY; FULL OUTER retains an independent COALESCE result. Added FE tests cover grouping, invalid nullable-side/FULL references, repeated CTE positions and an FE-computed (1,10,1) result.
- Scope and reuse: changes are focused and reuse existing slot-copying and ordered-map APIs; the CTE repair directly supports the identity change.
- Concurrency and locks: these query-local transforms introduce no thread, shared-state or locking changes.
- Lifecycle: creation, copying, pruning, nullable/type/ID/variant rewriting, both inliners and physical lowering were traced; no new resource or static lifetime is introduced.
- Configuration: no configuration or dynamic-setting behavior changes.
- Compatibility: no external symbol, persisted-format or wire-format changes were introduced.
- Thrift/protobuf: no changes; field-ID compatibility checks are not applicable.
- Parallel paths: inner, left/right outer/semi/anti, FULL, qualified/unqualified stars, chained keys and consumer rebuild/materialization paths were checked. No new rule registration/property alternative or mark-join change.
- Conditions: the existing FULL-computed/right-preserved/left-preserved selection remains justified and documented; opposite-side identities remain distinct.
- Test coverage: positive, negative and full-planner FE cases are present. The added regression checks EXPLAIN success. This invocation ran no builds or tests, as required by the review contract.
- Expected results: [x,y,z] and (1,10,1) match the intended positional semantics; no regression result file was changed.
- Observability: existing analysis errors and plan output suffice for these local fixes; no new logging or metric requirement was identified.
- Persistence and transactions: no EditLog, replay, visible-version, delete-bitmap or transaction path changes.
- Writes and atomicity: no storage-write or commit mechanism changes; query identity and output positions were the applicable correctness concerns.
- FE-BE variables: no new transmitted field or variable; existing ID mapping and projection translation were traced.
- Performance: forward-entry rebuilding removes repeated reverse-group traversal for repeated producers; no demonstrated performance regression.
- Other concerns: identity collisions, per-slot metadata, SQL-index preservation and positional downstream consumers were checked; no unresolved code candidate remains.
No additional user focus was supplied. Validation limits: this is static analysis, and the author's reported test/build results were not independently executed. The new tests do not add BE aggregate-result, forced-materialization or variant-order execution assertions; those paths were inspected in source. No changed path remains unreviewed, and these validation limits are not presented as verified defects.
Review converged after 1 round(s).
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 28263 ms |
Contributor
TPC-DS: Total hot run time: 150467 ms |
Contributor
ClickBench: Total hot run time: 24.19 s |
Contributor
TPC-H: Total hot run time: 28815 ms |
Contributor
TPC-DS: Total hot run time: 149961 ms |
Contributor
ClickBench: Total hot run time: 24.17 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
2 similar comments
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 28212 ms |
Contributor
TPC-DS: Total hot run time: 150296 ms |
Contributor
ClickBench: Total hot run time: 24.28 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
2 similar comments
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
yiguolei
pushed a commit
that referenced
this pull request
Oct 22, 2026
…ys (branch-4.2) (#69389) Backport of #69388 to branch-4.2. The production repair is unchanged; the backported tests use Java 8-compatible APIs. ### What problem does this PR solve? Issue Number: N/A Related PR: #69388 A qualified reference to a preserved-side USING key can be rejected even when the same key appears unqualified in GROUP BY. For example: ```sql SELECT t1.a, COUNT(*) FROM t1 LEFT JOIN t2 USING(a) GROUP BY a; ``` The USING-key projection creates a fresh Alias for a column forwarded directly from the preserved side. SELECT t1.a references the original ExprId while GROUP BY a references the new one. The same mismatch affects HAVING and ORDER BY. Keep the input ExprId for forwarded USING keys while clearing the qualifier of the unqualified view. Preserve qualified outputs and the explicit asterisk output list. FULL OUTER JOIN still builds a new COALESCE expression with its own ExprId, and aggregate validation remains unchanged. Sharing the forwarded key identity can repeat a producer ExprId in CTE output, for example: ```sql WITH q(x,y,z) AS ( SELECT a,t1.b,t1.a FROM t1 JOIN t2 USING(a) ) SELECT * FROM q; ``` Keep CTE consumer columns in their original positions using the ordered consumer-to-producer map. Flattening the reverse multimap groups equal producer slots and incorrectly changes [x,y,z] into [x,z,y]. Preserve the ordered map when updating nullability, replacing producer ExprIds and pruning variant subpaths, so those rewrites cannot reintroduce the ordering defect. ### Release note Fix spurious GROUP BY errors when qualified and unqualified references to a forwarded USING join key are mixed in SELECT, GROUP BY, HAVING, or ORDER BY. Preserve CTE output column order when the same source column appears more than once. ### Check List (For Author) - Test: Unit Test - Official run-fe-ut.sh: 73 tests, 0 failures/errors across BindUsingJoinTest, LogicalCTEConsumerTest, AnalyzeCTETest and EliminateGroupByKeyTest. - Reproduced the CTE column-order failure before the CTE repair: expected [x,y,z], actual [x,z,y]. - USING tests cover complete planning across retained sides, mixed grouping references, HAVING/ORDER BY, expressions, multiple keys, derived inputs and invalid nullable-side/FULL references. - Added CTE column-order cases for inner/left/right USING joins, an actual FE-computed result check for (1,10,1), and repeated-producer tests covering consumer copies, pruning, nullability changes and producer-ID replacement. - Added an EXPLAIN regression case to test_using_join_merge_key; the same SQL is covered by the FE full-planner test. The regression suite and BE data execution have not been run locally. - FE server build through DISABLE_BUILD_UI=ON ./build.sh --fe -j4, integrated Checkstyle, and git diff --check. Unmodified Web UI excluded. - Behavior changed: Yes, valid grouping queries plan successfully and CTE column positions remain intact. - Does this need documentation: No.
Contributor
Author
|
/review |
morrySnow
approved these changes
Oct 26, 2026
Contributor
There was a problem hiding this comment.
Static review of all eight changed files at head 363d34b70578aee08231e8082b6a0151ce199ac0 found no new substantiated findings.
Existing P1 comment 4334911111 was independently revalidated as fixed. Each producer position receives a distinct consumer ExprId, and LogicalCTEConsumer.computeOutput() now reads the ordered consumer-to-producer keys. The nullable, producer-ID and variant rebuilds preserve that order; inlining and materialized CTE translation use explicit identity mappings and ordered projections. The original interleaved [x,y,z] case no longer follows the reported [x,z,y] binding path, so this comment is not carried as an existing blocker or reposted.
Critical checkpoint conclusions:
- Goal and supporting tests: forwarding preserved-side USING keys with their input ExprId makes qualified and unqualified references match in GROUP BY, HAVING and ORDER BY. FULL OUTER keys retain a separate COALESCE Alias. Added planner assertions and CTE order/result assertions match these goals; they were inspected, not executed here.
- Scope and reuse: the five production files and three test files form a focused identity/order repair using existing slot and ordered-map APIs.
- Concurrency and locks: no concurrency or locking behavior is changed; these are staged, query-local plan transformations.
- Lifecycle and initialization: fresh per-position consumer IDs, immutable map copies and existing plan-copy lifecycles are retained; no resource or static-initialization change.
- Configuration: no configuration item or dynamic-update path is changed.
- Compatibility: no public symbol signature, storage format or RPC contract is changed; the intended observable changes are query acceptance and column ordering.
- Thrift/protobuf: no gensrc definitions change, so cross-branch field-ID checks are inapplicable.
- Parallel paths: reviewed retained left/right join sides, semi/anti, FULL and chained joins, qualifier/star binding, CTE construction/copy/pruning, nullable/ExprId/type/variant rebuilding, inlining, materialization, and positional UNION/INSERT mapping. No separate defect was established.
- Conditions and errors: no new conditional guard is added. Existing preserved-side selection remains appropriate; nullable-opposite-side and FULL grouping references retain distinct identities and existing aggregate validation.
- Test coverage: added cases cover all seven forwarding join forms, both grouping-reference directions, HAVING/ORDER BY, arithmetic, multiple keys, derived inputs, invalid opposite-side/FULL references, inner/left/right CTE order, consumer copies/pruning/nullability/ID replacement and an FE-computed repeated-column result. The new Groovy case checks EXPLAIN acceptance.
- Expected results:
[x,y,z], repeated producer identity, nullable positions and the literal result(1,10,1)agree with the traced semantics. No golden output file is added or changed. - Observability: existing analysis errors and EXPLAIN remain appropriate; no new operation requires additional metrics or logging.
- Transactions and persistence: no EditLog, replay, version-visibility or failover path is changed.
- Writes and crash behavior: no data-write, atomicity or resource-release path is changed.
- FE/BE propagation: no new variable or protocol field is introduced. CTE physical output mappings and multicast projection/tuple order were traced explicitly.
- Performance: ordered output remains linear in consumer count; the new entry-based visitors avoid repeatedly visiting reverse-map groups for repeated producers. No introduced performance defect was found.
- Other correctness checks: slot name/qualifier handling, outer-join nullability, duplicate output positions and map consistency were checked. No unresolved source-level candidate remains; committed-data visibility, delete bitmaps and native memory-accounting mechanisms are untouched.
No additional user-provided focus was supplied. Coverage is complete for the static review scope. No builds, tests, or BE execution were performed. The new tests do not demonstrate BE execution of a materialized USING/variant CTE, and the FE constant-result test does not contain a USING join; execution coverage of those combinations remains a validation limitation. Static tracing resolved the associated source-level risk questions without treating those tests as execution proof.
Review converged after 1 round(s).
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.
What problem does this PR solve?
Issue Number: N/A
Related PR: #64718
A qualified reference to a preserved-side USING key can be rejected even when the same key appears unqualified in GROUP BY. For example:
The USING-key projection creates a fresh Alias for a column forwarded directly from the preserved side. SELECT t1.a references the original ExprId while GROUP BY a references the new one. The same mismatch affects HAVING and ORDER BY.
Keep the input ExprId for forwarded USING keys while clearing the qualifier of the unqualified view. Preserve qualified outputs and the explicit asterisk output list. FULL OUTER JOIN still builds a new COALESCE expression with its own ExprId, and aggregate validation remains unchanged.
Sharing the forwarded key identity can repeat a producer ExprId in CTE output, for example:
Keep CTE consumer columns in their original positions using the ordered consumer-to-producer map. Flattening the reverse multimap groups equal producer slots and incorrectly changes [x,y,z] into [x,z,y]. Preserve the ordered map when updating nullability, replacing producer ExprIds and pruning variant subpaths, so those rewrites cannot reintroduce the ordering defect.
Release note
Fix spurious GROUP BY errors when qualified and unqualified references to a forwarded USING join key are mixed in SELECT, GROUP BY, HAVING, or ORDER BY. Preserve CTE output column order when the same source column appears more than once.
Check List (For Author)