[fix](fe) Fix DPHyp join reorder producing wrong results for aliases on nullable side of outer join

[starocean999]

Related PR: #61146

Problem Summary:

When enable_dphyp_optimizer=true, DPHyp join reorder can produce wrong results for queries where the nullable side of an outer join contains Alias/Project expressions with functions like COALESCE, IFNULL, or CAST(COALESCE(...)).

Root Cause:

HyperGraph.Builder.addAlias() unconditionally adds all aliases to aliasReplaceMap, including those defined on the nullable side of outer joins. This causes their defining expressions to be "unwrapped" and later reconstructed by PlanReceiver.proposeProject() ABOVE the outer join via finalProjects. The expression then operates on null-extended values, changing semantics — e.g., COALESCE(NULL, 0) = 0 instead of the correct NULL for non-matching outer join rows.

Fix:

Added an isNullableSide context flag that propagates through buildForDPhyper() and addAlias() during graph construction. When processing an outer join:

LEFT OUTER JOIN → right child flagged as nullable
RIGHT OUTER JOIN → left child flagged as nullable
FULL OUTER JOIN → both children flagged as nullable
The flag propagates through nested projects and inner joins
Aliases on the nullable side are not added to aliasReplaceMap, preserving the original Project boundary below the outer join so that expressions execute before null-extension.

None

Check List (For Author)

• Test

  • Regression test
  • Unit Test
  • Manual test (add detailed scripts or steps below)
  • No need to test or manual test. Explain why:
    • This is a refactor/code format and no logic has been changed.
    • Previous test can cover this change.
    • No code files have been changed.
    • Other reason

• Behavior changed:

  • No.
  • Yes.

• Does this need documentation?

  • No.
  • Yes.

Check List (For Reviewer who merge this PR)

• Confirm the release note
• Confirm test cases
• Confirm document
• Add branch pick label

[hello-stephen]

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?

[starocean999]

/review

[github-actions]

Automated review result: REQUEST_CHANGES

The simple one-leaf LEFT OUTER JOIN case is addressed, but I found three blocking planner-correctness gaps:

1. Nullable aliases are placed at their referenced-node bitmap rather than their original Project boundary, which changes nested outer-join null results and can move non-movable/volatile evaluation across inner joins.
2. Dependent nullable aliases sharing a bitmap are flattened into sibling expressions even though the later alias ExprId is absent from the LogicalProject child.
3. Multi-node aliases are emitted after their raw inputs have already been pruned from contributing DP groups.

The concrete plan trees and fix directions are in the inline comments.

Checkpoint conclusions:

• Goal and scope: the patch is focused and its new ordered regression results are internally consistent for the narrow leaf case, but the generalized placement structure does not preserve Project boundary, dependency layers, or pending-input liveness, so it does not safely accomplish the full goal.
• Join semantics, special conditions, and parallel paths: simple LEFT/RIGHT/FULL nullable-side propagation is directionally symmetric; nested joins and multi-node subsets fail as noted. ASOF checks are unreachable because this builder rejects ASOF joins. JoinOrderJob uses the v2 implementation; the legacy receiver is not a parallel runtime path, but it demonstrates the missing dependency/liveness handling.
• Tests and results: all four added queries exercise the same single-leaf LEFT JOIN shape. Nested outer, multi-node, dependent-alias, non-movable/volatile, RIGHT, and FULL cases are not covered; the first four are tied to concrete failures above.
• Concurrency, lifecycle, configuration, compatibility, persistence, transaction/data-write behavior, FE-BE transport, and observability: not applicable to this per-query FE planner change; no new shared runtime state, config, protocol, or persisted data is introduced.
• Performance: no separate substantiated performance regression was found.
• Validation: static review only, as required by the review runner; no build or test command was run.
• User focus: no additional user-provided focus was supplied.

Review completion: the full four-file change and related planner paths were reviewed through two convergence rounds; every candidate was resolved and all final reviewers returned NO_NEW_VALUABLE_FINDINGS against this exact comment set.

[starocean999]

/review

[github-actions]

Automated review result: REQUEST_CHANGES

The follow-up fixes the simple leaf nullable-alias case, but three distinct planner-correctness gaps remain:

1. A retained alias can activate an upper join edge at its smaller input bitmap before the full source Project subtree emits the alias, producing a join predicate whose slot is absent from both children.
2. Chained replacement bypasses the normal volatile-expression merge guard and can evaluate a volatile producer more than once, changing values.
3. The same transitive expansion bypasses the normal expression-limit fallback, so DPHyp graph construction can reject a valid layered plan near the configured limit.

The earlier Project-boundary, dependent-alias, and pending-input threads remain the authoritative locations for their unresolved constant/interleaved variants; duplicate inline comments were suppressed.

Checkpoint conclusions:

• Goal and scope: the four-file patch is focused and the basic unmatched LEFT OUTER case is fixed, but the edge-availability, volatile, and expression-limit failures mean the generalized goal is not yet accomplished.
• Join semantics, special conditions, and parallel paths: LEFT/RIGHT/FULL nullable propagation is symmetric; ASOF is rejected before these checks, and mark/leading/hinted joins remain opaque. JoinOrderJob uses hypergraphv2, so no active parallel receiver change is missing. Inner association and partial DP alternatives expose the failures above.
• Tests and results: the checked-in aggregates are arithmetically consistent and use ordered queries. However, the deterministic chained case merges before DPHyp and the multi-node case never consumes ab.sv, so neither exercises its claimed mechanism. Parent alias predicates, non-mergeable volatile chains, actual multi-node liveness, RIGHT/FULL, matched nullable rows, and the expression-limit fallback are not proved.
• Concurrency and lifecycle: not applicable; the added maps are per-query optimizer state used synchronously, with no threads, locks, static initialization, or resource ownership changes.
• Configuration and compatibility: no config definition, persisted format, EditLog, FE-BE field, RPC symbol, rolling-upgrade, or storage contract changes are introduced.
• Transactions, data writes, crash recovery, and observability: not applicable to this logical FE planner change; no production write path or distributed operation is modified, and no new logs or metrics are needed.
• Error handling and performance: invalid alias availability can fail rewrite validation, and expression-limit exceptions escape the normal merge fallback. Alias-map scans and wider intermediate projections add overhead, but no separate performance-only blocker was substantiated.
• Other core invariants: data visibility, versions, delete bitmaps, BE memory/nullable-column handling, and security are outside this planner-only change.
• Validation: static review only, as required by the review runner; no build or test command was run.
• User focus: no additional user-provided focus was supplied.

Review completion: the complete four-file patch, related planner control flow, tests, existing threads, and all candidate points were reviewed through three allowed convergence rounds. Every candidate was accepted, dismissed with code evidence, or deduplicated, and the final normal and risk-focused reviewers returned NO_NEW_VALUABLE_FINDINGS against this exact comment set.

[starocean999]

/review

[github-actions]

Automated review result: REQUEST_CHANGES

The latest revision addresses the previously reported boundary, dependency, volatility, expression-limit, pending-input, and alias-consumer mechanisms, but it introduces one distinct planner-correctness failure: each rebuilt source Project layer contains only aliases, so applying it as a new LogicalProject discards pass-through outputs such as the nullable child's join key. The parent join can then reference a slot absent from its child and fail rewrite validation. The concrete plan tree and fix direction are in the new inline comment; the six existing threads remain the authoritative locations for their own mechanisms and were not duplicated.

Critical checkpoint conclusions:

• Goal and proof: the goal is to preserve nullable-side Project semantics during DPHyp reorder. The new layered representation is directionally appropriate, but alias-only layers drop required outputs, so the goal is not yet accomplished. The added row-result suite does not prove a valid reconstructed plan.
• Scope and clarity: the change is focused on HyperGraph/PlanReceiver plus one regression suite and generated output. The remaining issue is in the central reconstruction step, not unrelated code.
• Join semantics, conditions, and parallel paths: LEFT/RIGHT/FULL nullable propagation matches the null-extended side; inherited context through INNER/CROSS/SEMI/ANTI is intentional, and ASOF/mark/leading/hinted joins are excluded. The legacy receiver preserves pass-through and projected outputs together and confirms the missing v2 invariant.
• Tests and results: all checked-in expected aggregates are arithmetically consistent and ordered. Several advertised branches remain unproved (including the unused multi-node alias and deterministic/unmatched volatile case), but those coverage obligations are already present in the existing threads and were deduplicated.
• Concurrency and lifecycle: not applicable; this is synchronous per-query optimizer state with no locks, shared threads, unusual ownership, static initialization, or teardown changes.
• Configuration and compatibility: no configuration definition, persisted format, EditLog, RPC/FE-BE field, function symbol, rolling-upgrade, or storage-format change is introduced.
• Transactions, data writes, recovery, and observability: not applicable to this logical FE planner change; no production write path or distributed operation changes, and no new logging/metrics requirement was identified.
• Error handling and performance: the missing child slot is a hard planner-validation failure. Repeated alias-map scans are bounded by the existing DPHyp enumeration limit; no separate performance blocker was substantiated.
• Other core invariants: data visibility, versions, delete bitmaps, BE memory/nullable-column handling, and security are outside this planner-only patch.
• Validation: static inspection only, as the review runner explicitly forbids builds and test execution.
• User focus: no additional user-provided review focus was supplied.

Review completion: the complete four-file patch, related planner control flow, tests, existing threads, and every candidate were reviewed through two convergence rounds. Both final normal reviewers and the separate risk-focused reviewer returned NO_NEW_VALUABLE_FINDINGS for this exact ledger and comment set.

[starocean999]

/review

[github-actions]

Automated review result: COMMENT (no new non-duplicate inline findings)

The current head fixes the alias-only Project-layer pass-through failure at the exact stored boundary, but the full review still reproduced two broader materialization-boundary failures: crossing partial DP groups can prune pending alias inputs, and a missed edge can consume an alias before the post-join layer emits it. I did not add duplicate inline comments because those invariants are already stated by the existing pending-input, consumer-ownership, and source-Project-boundary threads; those threads remain authoritative for the unresolved blockers.

Critical checkpoint conclusions:

• Goal and proof: the patch is focused on preserving nullable-side aliases during DPHyp reorder, and exact-boundary layer ordering/pass-through is improved. The goal is not yet proven complete because legal crossing subsets still violate the previously requested “until emission” and source-stage invariants.
• Join semantics, conditions, and parallel paths: LEFT/RIGHT/FULL nullable-side selection is directionally symmetric; ASOF, mark, leading, and hinted joins are excluded from this builder. Same-key chains, constants, volatile expressions, expression limits, ExprIds, final projection, and the active v2/legacy-reference paths were traced. The remaining crossing/missed-edge routes are covered by existing threads.
• Tests and results: the ordered .out values are arithmetically consistent and the basic unmatched leaf case is effective. The multi-node alias is unused, the “volatile” SQL has no volatile function or match, the nested case has no inner null-extension, and FE tests assert only plan construction; these gaps correspond to existing review requests and were deduplicated.
• Concurrency and lifecycle: not applicable; the new structures are synchronous per-query optimizer state with no locks, threads, static initialization, ownership, or teardown changes.
• Configuration, compatibility, persistence, transactions/data writes, FE-BE transport, security, and observability: not applicable; no config, protocol, persisted state, write path, trust boundary, log, or metric is changed.
• Error handling and performance: the unresolved existing paths can build joins/Projects with missing child slots and fail validation. Repeated alias-layer scans are bounded by the existing DPHyp enumeration limit; no separate performance-only blocker was substantiated.
• Validation: static inspection only, as the review runner forbids builds and test execution. Current GitHub CheckStyle is green; no functional test result was independently run here.
• User focus: no additional user-provided review focus was supplied.

Review completion: the complete five-file patch, related planner paths, live threads, and all candidates were reviewed through two convergence rounds. Every candidate was accepted, dismissed with evidence, or deduplicated, and the final normal and risk-focused reviewers returned NO_NEW_VALUABLE_FINDINGS against this exact ledger and comment set.

[starocean999]

/review

[github-actions]

Request changes: two existing P1 discussions still apply to the current head.

1. Pending alias inputs must survive until emission. For a nullable-side Project(A.v + B.v AS s) over {A,B}, the new helper retains inputs for direct subsets such as {A}, but not for a legal mixed connected subplan {A,C}. Inner/inner reordering can build {A,C} first, prune A.v, then join B; the {A,B} layer is emitted over {A,B,C} with its input already missing. This is the same existing "until emission" contract, so I did not post a duplicate inline.

2. The source Project boundary still is not encoded. Keying a layer by {A,B} does not prevent it from first being emitted on a wider {A,B,C} union. A zero-input uuid() alias originally evaluated over A JOIN B can therefore be moved above an unrelated C join, changing evaluation multiplicity; a none-movable assertion can similarly move past filtering. Separate layers fix chained flattening, but not this whole-layer placement problem. This is already covered by the existing boundary thread, so no duplicate inline was added.

Checkpoint conclusions:

• Goal and proof: the direct leaf-keyed COALESCE/IFNULL, consumer-bitmap, expression-limit, chained-layer, and pass-through cases are addressed, but the arbitrary-reordering cases above mean the full correctness goal is not yet met.
• Scope and design: the production changes are localized to DPHyp alias bookkeeping and reconstruction. No new concurrency, lifecycle, configuration, persistence/EditLog, transaction/data-write, FE/BE protocol, or observability concern applies.
• Parallel paths: LEFT/RIGHT/FULL nullable propagation is symmetric in code; ASOF joins are excluded before the added ASOF checks. Tests cover only LEFT OUTER joins.
• Error handling and performance: invariants still fail loudly through preconditions/CheckAfterRewrite; no distinct error-boundary or substantiated performance issue was found beyond the wrong-stage evaluation above.
• Tests: expected .out arithmetic is deterministic and internally correct, but sv is never consumed, the "volatile" test is deterministic and has no matching row, and the nested case never exercises inner null-extension. The non-null/result-only oracles can also pass if DPHyp falls back to the original group. These gaps support the regression requirements in the existing threads rather than warranting duplicate comments.

There was no additional user-provided review focus. Review was static only, as required by the runner contract; no build or test command was run, and local thirdparty/installed/bin/protoc is unavailable. The current GitHub CheckStyle check is passing.

[starocean999]

/review

[github-actions]

Automated review result: REQUEST_CHANGES

The latest head addresses the same-key dependency layers, expression-limit fallback, direct mixed-input retention, alias pass-through, and ordinary consumer-bitmap cases, but two existing P1 correctness threads still apply:

1. The source Project boundary is still not encoded. If DPHyp builds {A,C} before B, a layer sourced at {A,B} is first emitted on {A,B,C}; volatile/non-movable evaluation can therefore move across C's filtering or row multiplication.
2. A consumer can still run before its alias is emitted. In the same mixed split, processMissedEdges can attach the {A,B}--C predicate to the join of {A,C} and B, while proposeProject emits the alias only after that join.

I added two distinct P2 inline comments for current-head performance regressions: raw alias inputs remain live through every ancestor after materialization, and bare forwarding aliases are unnecessarily rebound to full source subtrees, eliminating safe selective join orders.

Critical checkpoint conclusions:

• Goal and proof: the patch is focused on preserving nullable-side Project semantics during DPHyp reorder, but the two existing P1 paths above mean the full correctness goal is not achieved.
• Join semantics, special conditions, and parallel paths: LEFT/RIGHT/FULL nullable-context propagation is symmetric in code; ASOF, mark, leading, and hinted joins are excluded before these paths. Mixed inner subplans and the missed-edge fallback expose the remaining failures.
• Tests and results: the ordered .out values are internally consistent, but the mixed alias is unused, the claimed volatile case is deterministic and unmatched, the nested case never observes inner null-extension, and FE tests assert only non-null plan construction even though DPHyp may fall back to the original group. RIGHT/FULL and the two new performance invariants are untested.
• Concurrency and lifecycle: not applicable; the added maps/lists are synchronous per-query optimizer state with no locks, threads, static initialization, ownership, or teardown change.
• Configuration, compatibility, persistence, transactions/data writes, FE-BE transport, and observability: not applicable; no config, protocol, persisted state, write path, or distributed runtime surface changes.
• Error handling and performance: the existing P1 paths can produce a join predicate bound to a missing child slot or change volatile/non-movable evaluation. The two inline P2 findings cover intermediate-width and search-space regressions.
• Validation: static review only, as required by the review runner. GitHub CheckStyle is green, but no targeted FE unit or regression result is available here; thirdparty/installed and protoc are absent.
• User focus: no additional user-provided review focus was supplied.

Review completion: the complete five-file patch, related join-order control flow, live threads, test oracles, and every candidate were reviewed through three convergence rounds. Every candidate was accepted, dismissed with code evidence, or deduplicated, and the final normal and risk-focused reviewers returned NO_NEW_VALUABLE_FINDINGS against this exact payload.

[starocean999]

/review

[github-actions]

Automated review result: REQUEST_CHANGES

The patch improves ordinary nullable-side Project reconstruction, but three P1 planner-correctness failures remain:

1. The bare-Slot endpoint exception exposes an alias to join enumeration without substituting predicate consumers, so a normalization-stable mixed Project can produce a join predicate whose alias slot is absent from both children.
2. A lower stored Project layer carries only final/edge requirements and can drop a raw slot needed by a later join-separated Project layer.
3. A Project layer keyed to its source subtree may first be emitted only on a larger reordered supertree, moving volatile or non-movable evaluation across an unrelated join.

The concrete plan trees and fix directions are in the inline comments.

Critical checkpoint conclusions:

• Goal and proof: the current implementation does not fully preserve nullable-side Project semantics for legal DPHyp alternatives, and the added tests do not cover the three failing shapes.
• Scope and clarity: the patch is localized, but the layered materialization model needs explicit alias-availability, inter-layer slot-lifetime, and exact-source-boundary invariants.
• Join semantics and parallel conditions: LEFT/RIGHT/FULL nullable-context propagation and nested inner alternatives were reviewed; ASOF joins are rejected by isValidJoin, and mark, hinted, and leading joins do not enter this builder. The bare-Slot exception and containing-supertree eligibility remain insufficiently constrained.
• Error handling and data correctness: two paths construct expressions whose input slots are absent from child outputs; the third changes volatile/error evaluation stage and cardinality. These are planner-correctness failures rather than recoverable status paths.
• Tests and results: the checked-in ordered results are deterministic and internally consistent, but the multi-node alias is never consumed, the claimed volatile case is deterministic and unmatched, and plan-only non-null assertions do not prove forwarding reorder, inter-layer lifetime, or exact source-stage behavior.
• Concurrency and lifecycle: no threads, locks, shared mutable runtime state, static initialization, resource ownership, or teardown changes are introduced.
• Configuration, compatibility, persistence, transactions, data writes, FE-BE transport, security, and observability: not applicable to this FE-local logical optimizer change; no such boundary is modified.
• Performance: no new standalone performance blocker beyond existing review threads; correctness-preserving materialization constraints must precede the endpoint/search-space optimization.
• Validation: static inspection only, as required by the review runner; no build or test command was run.
• User focus: no additional user-provided review focus was supplied.

Review status: incomplete at the explicit three-round cap. The final round required two material reproducer/context corrections; I independently verified and applied those corrections, but the cap prohibited another convergence pass on the exact final payload. Per the cap exit rule, this review submits the corrected confirmed blockers without claiming full convergence or that no other issue is possible. The live head at submission was 602425d4a4d2ed221783c329c2e5d460c16bed67.

[starocean999]

/review

[github-actions]

Review outcome: request changes for 3dfb8ceb8a6d0b0651e28a08932fe8b93b2937f1.

The current head still allows a preserved nullable-side Project layer to materialize above its exact source boundary. In the feasible {A,C} + {B} order, a layer sourced at {A,B} first satisfies getProjectedAliasLayers only at {A,B,C}; an outside multiplying/filtering relation can therefore change volatile evaluation count or suppress a non-movable error. This is already captured by the live current-head P1 thread, so I did not post a duplicate inline comment.

Critical checkpoint conclusions:

• Goal and correctness: the direct null-extension bug for simple nullable-side aliases is addressed, but exact source-boundary preservation is not complete because of the reordered plan above.
• Scope and clarity: the change is localized to DPHyp graph metadata, Project reconstruction, and tests, but the new layer/liveness machinery is broader than the original alias-replacement condition.
• Concurrency: not involved; graph construction and receiver enumeration use job-local mutable state.
• Lifecycle/static initialization: not involved; no new long-lived resources, cycles, threads, or cross-TU state.
• Configuration: no configuration item or dynamic-reload behavior is added.
• Compatibility: no FE/BE protocol, persisted format, function symbol, rolling-upgrade, or storage compatibility boundary changes.
• Parallel paths and join types: LEFT, RIGHT, and FULL OUTER nullable propagation matches their null-extending children. ASOF, mark, leading, and hinted joins remain atomic at this builder boundary; semi/anti descendants introduce no separate null-extension. The prior forwarding/missing-input current-head variants were rechecked and no longer reproduce.
• Conditional/error handling: the nullable-context conditions are consistent with reachable join types. The remaining error is the layer-eligibility predicate, which accepts a source key inside a larger newly formed union instead of enforcing its exact materialization barrier.
• Test coverage: the simple unmatched COALESCE/IFNULL/CAST regressions detect upward movement across the outer join, and all ten .out values are arithmetically consistent. Coverage is incomplete for the live P1: no test forces the mixed reordered boundary with a volatile or non-movable expression. The multi-node alias is not consumed, the purported volatile case is deterministic and unmatched, the nested case has no missing inner row, and Java tests only assert that plan construction succeeds. These gaps were already requested in existing threads, so they were not reposted.
• Test results: this review runner was explicitly review-only, so I did not build or run tests. The live checks observed for this head include successful CheckStyle/format/license checks, but no completed FE unit or regression result was used as correctness evidence.
• Observability: no runtime path requiring new logs, metrics, or tracing is introduced.
• Persistence/transactions/data writes: not involved; there are no EditLog, failover, transactionality, atomicity, or crash-recovery changes.
• FE/BE variable passing: no new transmitted variable or scattered thrift/protobuf send path.
• Performance: the current global alias-input set retains already-consumed raw columns through later joins until the final Project, matching the already-known P2 thread. No distinct performance finding was added.
• Other issues: after full changed-file and upstream/downstream sweeps, no additional non-duplicate correctness issue was substantiated.

There was no additional user-provided focus. Review convergence is complete for this head: the main-agent scan, two complete-review agents, and the separate risk-focused agent all reviewed the same final ledger/comment set and returned no new valuable findings after deduplication. The review itself is complete; the PR remains blocked on the existing exact-boundary P1.

[starocean999]

/review

[github-actions]

Automated review conclusion: request changes

I found one new, non-duplicate P1 correctness issue on head 3fd9ab1cd9226ec9d31a5efb874b1491f72d5c9f: a nullable-side alias mapped to its full source bitmap can still be consumed before materialization when DPHyp splits that bitmap across two children and processMissedEdges attaches the alias predicate. The inline comment contains the concrete plan and fix direction.

Critical checkpoint conclusions:

• Goal and data correctness: the change preserves basic COALESCE/IFNULL/CAST aliases below an ancestor outer join, but the split-hyperedge alternative can build a join predicate with a missing alias ExprId, so the stated correctness goal is not complete.
• Scope and parallel paths: the code is FE/Nereids-local. LEFT/RIGHT/FULL nullable propagation, nested joins, constant/forwarding/complex/chained aliases, mark/leading/hinted/ASOF boundaries, and the volatile/NoneMovableFunction cluster boundaries were reviewed. No second distinct issue survived deduplication.
• Concurrency and lifecycle: no shared-thread state, lock ordering, static initialization, or special resource lifecycle is introduced.
• Configuration, compatibility, persistence, and writes: no configuration, FE-BE protocol, storage format, rolling-upgrade, EditLog, transaction, or data-write path is changed.
• Tests and results: the regression output arithmetic is internally consistent and covers the basic LEFT nullable-side cases, but no changed test covers a consumed {A,B} alias plus an independently connecting C, which is needed for the reported failure. The FE tests assert successful planning rather than Project placement or child-output ExprIds. Per the review-runner contract I did not build or run tests locally; the current PR checks show CheckStyle passing, but no FE unit/regression execution was observed in the listed checks.
• Performance: raw alias-input retention and forwarding-endpoint reorder loss were reviewed as existing-thread concerns and were not duplicated here. No additional performance finding was substantiated.
• Observability and error handling: no new runtime-observability requirement applies; the accepted issue violates the planner child-output invariant rather than adding a new error boundary.

User focus: no additional focus points were provided.

Review completion: three rounds covered the full changed-file set, including two normal full-review agents and a separate risk-focused agent per round. Every candidate was independently verified and either accepted for the inline comment or suppressed with concrete duplicate/code evidence.

[starocean999]

/review

[github-actions]

Request changes: one additional P1 correctness issue remains in the nullable-side alias-layer reconstruction. A join-separated computed alias can still reference a direct forwarding alias whose producer the builder removed, so DPHyp can construct a LogicalProject with an input ExprId absent from its child and reject a valid query.

Critical checkpoint conclusions:

• Goal and correctness: the change preserves computed aliases below outer-join null extension in the covered cases, but the forwarding-to-computed transition in MAIN-001 leaves the end-to-end fix incomplete.
• Scope and clarity: the production changes and tests are focused on DPHyp v2 nullable-side Project handling; no unrelated source changes were found.
• Concurrency and lifecycle: this is planner-local memo/enumeration state with no new threads, shared static state, lock ordering, or special resource lifecycle.
• Conditions and parallel paths: normal CSG/CMP edges, missed-edge fallback, graph simplification, LEFT/RIGHT/FULL nullable propagation, direct forwarding, computed layers, volatile/NoneMovable barriers, and the old parallel hypergraph implementation were inspected. ASOF joins remain outside this v2 join-cluster path. Other live concerns are already covered by existing inline threads and were not duplicated.
• Configuration, compatibility, persistence, and data writes: no new configuration, FE-BE protocol/storage format, EditLog, transaction, or data-write behavior is introduced.
• Tests and results: FE unit and regression coverage were added, but there is no join-separated forwarding-to-computed regression for MAIN-001. The vacuous volatile, multi-node, and nested-outer cases map directly to existing threads 3594102562, 3592512016, and 3592512010. Per the review-runner contract, no build or test execution was attempted; this checkout is also not worktree-initialized and lacks thirdparty/installed/bin/protoc.
• Performance and observability: no new observability requirement was identified. Over-retention of raw alias inputs remains the known issue in thread 3600123245; no duplicate comment was submitted.
• User focus: no additional user-provided focus was supplied.

Overall: request changes for MAIN-001; all other suspicious points have been either tied to existing review context or dismissed with code evidence.

[starocean999]

/review

[github-actions]

Automated code review: changes requested.

I found one new P2 issue in the added coverage: the mixed-subplan test can prune the alias before DPHyp, has no third relation inside the nullable subquery to create the claimed partial-overlap alternative, and accepts original-plan fallback. See the inline comment on NullableAliasTest.java.

Critical checkpoint conclusions:

• Goal and proof: The production changes preserve complex nullable-side aliases as ordered Project layers, keep volatile and non-movable Projects as cluster boundaries, and gate split-source missed edges before proposeJoin. The main left-outer result cases are covered, but the newly added mixed-subplan test does not prove the required alias-input lifecycle, so the PR is not yet fully demonstrated by its tests.
• Scope and clarity: The implementation is focused on HyperGraph/PlanReceiver plus targeted tests. The layer/endpoint design is necessarily intricate but follows the existing DPHyp flow; no unrelated runtime behavior is changed.
• Parallel paths and special conditions: Left, right, and full outer nullable propagation are symmetric; ASOF joins remain opaque; regular and graph-simplified edges require the complete alias endpoint in one child; the new safety check covers the distinct missed-edge split path. Forwarding aliases are substituted consistently, and post-DPHyp AdjustNullable repairs nullability before CheckAfterRewrite.
• Concurrency and lifecycle: This is single-threaded optimizer construction/reconstruction. No shared mutable runtime state, lock ordering, asynchronous lifecycle, or static-initialization concern applies.
• Compatibility, persistence, and data flow: No configuration, storage/serialization format, EditLog, transaction, data-write, FE-BE transport, or rolling-upgrade boundary is changed.
• Error handling and invariants: Existing preconditions plus the final nullable/output checker keep invalid ExprId/output shapes loud. The test's original-group fallback is the verification weakness identified inline, not a new production error-swallowing path.
• Tests and results: The regression result arithmetic is internally consistent and ordered. However, the mixed alias is unused in both the Java and regression cases, the Java checks only require a non-null plan, and the claimed mixed alternative is absent from the nullable subquery. Per the review-run contract I did not build or run tests locally. The visible GitHub checks include passing CheckStyle, but no targeted FE unit/regression result was available in this run.
• Performance and observability: The completed-layer raw-input retention concern is already covered by an existing inline thread and was not duplicated. No other distinct performance issue was substantiated; existing DPHyp tracing is adequate for this planner-only change.
• User focus: No additional user-provided focus was supplied.

All changed files and existing inline threads were reviewed and deduplicated. After the final wording was frozen, three independent review tracks converged with no additional valuable findings. Please address the inline test-validity issue before approval.

[starocean999]

/review

[starocean999]

run buildall

[hello-stephen]

TPC-DS: Total hot run time: 177676 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 5c4d02910ab30d37b789b105cd289ed409be8950, data reload: false

query5	4321	641	464	464
query6	462	216	207	207
query7	4961	600	329	329
query8	330	182	182	182
query9	8810	4070	4050	4050
query10	453	351	305	305
query11	5881	2328	2127	2127
query12	150	100	95	95
query13	1288	635	437	437
query14	6237	5175	4855	4855
query14_1	4205	4206	4221	4206
query15	213	202	177	177
query16	1061	503	445	445
query17	1137	713	572	572
query18	2752	474	340	340
query19	203	184	140	140
query20	116	106	106	106
query21	229	155	138	138
query22	13558	13545	13296	13296
query23	17526	16480	16153	16153
query23_1	16386	16345	16286	16286
query24	7601	1779	1294	1294
query24_1	1250	1260	1298	1260
query25	552	439	355	355
query26	1340	356	208	208
query27	2530	577	385	385
query28	4400	2018	1993	1993
query29	1041	615	462	462
query30	335	259	233	233
query31	1109	1096	985	985
query32	103	61	60	60
query33	539	318	247	247
query34	1190	1183	672	672
query35	763	771	676	676
query36	1024	1022	895	895
query37	154	107	90	90
query38	1879	1718	1651	1651
query39	864	859	858	858
query39_1	823	834	840	834
query40	244	171	144	144
query41	66	63	63	63
query42	95	90	95	90
query43	322	327	275	275
query44	1408	799	757	757
query45	190	179	174	174
query46	1142	1223	746	746
query47	2093	2123	2043	2043
query48	394	393	301	301
query49	578	436	310	310
query50	1046	443	364	364
query51	10732	10618	10606	10606
query52	90	93	92	92
query53	273	289	197	197
query54	289	246	245	245
query55	78	74	67	67
query56	336	318	297	297
query57	1327	1273	1213	1213
query58	293	273	255	255
query59	1583	1644	1426	1426
query60	328	295	255	255
query61	150	142	149	142
query62	540	500	437	437
query63	240	212	198	198
query64	2770	1031	856	856
query65	4758	4629	4626	4626
query66	1783	503	410	410
query67	29320	28714	29145	28714
query68	3291	1650	1072	1072
query69	423	313	266	266
query70	894	806	806	806
query71	367	347	320	320
query72	3021	2792	2638	2638
query73	838	791	461	461
query74	5051	4952	4718	4718
query75	2541	2494	2162	2162
query76	2362	1182	773	773
query77	367	389	288	288
query78	11875	11789	11381	11381
query79	1418	1158	781	781
query80	1338	578	503	503
query81	561	342	293	293
query82	571	156	121	121
query83	415	333	314	314
query84	298	170	185	170
query85	971	611	511	511
query86	405	249	223	223
query87	1828	1836	1730	1730
query88	3834	2860	2854	2854
query89	429	375	337	337
query90	1917	208	202	202
query91	197	185	162	162
query92	61	63	54	54
query93	1783	1582	993	993
query94	714	357	326	326
query95	798	499	482	482
query96	1019	813	378	378
query97	2692	2606	2497	2497
query98	211	216	208	208
query99	1083	1118	972	972
Total cold run time: 264130 ms
Total hot run time: 177676 ms

[hello-stephen]

ClickBench: Total hot run time: 24.81 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 5c4d02910ab30d37b789b105cd289ed409be8950, data reload: false

query1	0.00	0.00	0.00
query2	0.10	0.05	0.04
query3	0.25	0.13	0.13
query4	1.61	0.14	0.14
query5	0.24	0.22	0.23
query6	1.15	0.80	0.85
query7	0.04	0.01	0.01
query8	0.05	0.03	0.03
query9	0.38	0.31	0.34
query10	0.54	0.53	0.53
query11	0.20	0.13	0.13
query12	0.18	0.15	0.14
query13	0.47	0.48	0.48
query14	1.02	1.02	1.02
query15	0.62	0.61	0.61
query16	0.31	0.32	0.32
query17	1.16	1.10	1.08
query18	0.22	0.22	0.22
query19	2.08	1.89	1.89
query20	0.02	0.02	0.01
query21	15.47	0.19	0.14
query22	4.99	0.05	0.05
query23	16.11	0.31	0.12
query24	2.91	0.40	0.34
query25	0.11	0.05	0.06
query26	0.71	0.22	0.16
query27	0.04	0.03	0.04
query28	3.58	0.86	0.54
query29	12.57	4.17	3.33
query30	0.28	0.16	0.16
query31	2.76	0.59	0.31
query32	3.23	0.59	0.48
query33	3.16	3.19	3.15
query34	15.52	4.24	3.51
query35	3.54	3.55	3.51
query36	0.56	0.45	0.43
query37	0.09	0.07	0.06
query38	0.04	0.04	0.03
query39	0.04	0.03	0.03
query40	0.19	0.17	0.17
query41	0.09	0.04	0.03
query42	0.04	0.03	0.03
query43	0.05	0.04	0.04
Total cold run time: 96.72 s
Total hot run time: 24.81 s

[hello-stephen]

FE Regression Coverage Report

Increment line coverage 58.94% (89/151) 🎉
Increment coverage report
Complete coverage report

[starocean999]

/review

[starocean999]

run buildall

[github-actions]

Codex automated review failed and did not complete.

Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Aug 5th, 2026 4:09 AM.
Workflow run: https://github.com/apache/doris/actions/runs/30616202321

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

[hello-stephen]

TPC-H: Total hot run time: 29206 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit cc467bf83a8fcd2d081b58b1c68c1ccab2896f49, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17656	3978	3942	3942
q2	2065	361	202	202
q3	10204	1439	815	815
q4	4683	475	342	342
q5	7564	834	561	561
q6	181	176	137	137
q7	758	826	609	609
q8	9339	1595	1532	1532
q9	5420	4042	4064	4042
q10	6742	1661	1351	1351
q11	523	366	343	343
q12	727	621	456	456
q13	18144	3265	2774	2774
q14	272	258	245	245
q15	q16	737	738	667	667
q17	1006	989	1075	989
q18	7067	5711	5486	5486
q19	1308	1141	1094	1094
q20	842	636	598	598
q21	6346	2859	2694	2694
q22	461	391	327	327
Total cold run time: 102045 ms
Total hot run time: 29206 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4933	4589	4632	4589
q2	300	330	246	246
q3	4864	5294	4729	4729
q4	2226	2258	1455	1455
q5	4735	4383	4450	4383
q6	230	198	156	156
q7	1876	1755	1522	1522
q8	2344	2057	2054	2054
q9	7106	6720	6695	6695
q10	4306	4254	3802	3802
q11	512	376	344	344
q12	696	717	501	501
q13	2999	3349	2802	2802
q14	274	287	265	265
q15	q16	658	690	612	612
q17	1257	1231	1239	1231
q18	7234	6678	6757	6678
q19	1117	1077	1076	1076
q20	2213	2188	1913	1913
q21	5333	4665	4503	4503
q22	531	455	403	403
Total cold run time: 55744 ms
Total hot run time: 49959 ms

[hello-stephen]

TPC-DS: Total hot run time: 169634 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 cc467bf83a8fcd2d081b58b1c68c1ccab2896f49, data reload: false

query5	4301	582	462	462
query6	465	239	218	218
query7	4836	592	359	359
query8	322	178	171	171
query9	8804	4050	4049	4049
query10	495	353	298	298
query11	5858	2199	1978	1978
query12	155	101	97	97
query13	1237	590	426	426
query14	6150	5057	4379	4379
query14_1	3838	3816	3792	3792
query15	208	203	176	176
query16	1051	482	431	431
query17	1107	701	587	587
query18	2573	491	350	350
query19	209	190	154	154
query20	112	106	102	102
query21	243	169	139	139
query22	13046	13016	12763	12763
query23	17351	16385	16029	16029
query23_1	16207	16182	16180	16180
query24	7476	1705	1247	1247
query24_1	1242	1258	1279	1258
query25	529	441	351	351
query26	1329	362	213	213
query27	2619	561	385	385
query28	4424	2037	2056	2037
query29	1064	613	465	465
query30	346	263	232	232
query31	1109	1076	950	950
query32	108	62	58	58
query33	511	305	247	247
query34	1175	1143	656	656
query35	747	749	645	645
query36	780	763	708	708
query37	152	105	104	104
query38	1846	1645	1589	1589
query39	813	826	812	812
query39_1	784	770	826	770
query40	250	170	141	141
query41	72	80	81	80
query42	102	100	90	90
query43	318	321	268	268
query44	1422	791	765	765
query45	189	184	161	161
query46	1073	1174	721	721
query47	1513	1522	1490	1490
query48	420	426	293	293
query49	605	415	292	292
query50	1099	434	334	334
query51	10609	10624	10539	10539
query52	84	87	74	74
query53	266	261	197	197
query54	287	233	215	215
query55	72	70	70	70
query56	300	308	294	294
query57	1050	1015	917	917
query58	297	252	262	252
query59	1534	1635	1362	1362
query60	307	273	268	268
query61	151	148	153	148
query62	416	320	260	260
query63	242	190	198	190
query64	2876	1046	877	877
query65	3899	3866	3836	3836
query66	1826	487	385	385
query67	28332	27710	28245	27710
query68	3285	1523	993	993
query69	423	310	274	274
query70	895	805	780	780
query71	373	369	337	337
query72	3185	2841	2425	2425
query73	879	785	431	431
query74	4642	4493	4336	4336
query75	2381	2344	1978	1978
query76	2334	1138	752	752
query77	323	374	277	277
query78	11198	11262	10651	10651
query79	1337	1179	740	740
query80	739	550	472	472
query81	472	328	294	294
query82	564	150	115	115
query83	392	331	295	295
query84	289	162	131	131
query85	994	612	534	534
query86	382	229	241	229
query87	1823	1803	1706	1706
query88	3759	2828	2834	2828
query89	402	314	289	289
query90	1813	201	188	188
query91	201	215	169	169
query92	65	61	56	56
query93	1583	1501	972	972
query94	609	373	320	320
query95	774	579	469	469
query96	1114	780	349	349
query97	2461	2455	2351	2351
query98	208	202	194	194
query99	714	726	612	612
Total cold run time: 255826 ms
Total hot run time: 169634 ms

[hello-stephen]

ClickBench: Total hot run time: 24.01 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit cc467bf83a8fcd2d081b58b1c68c1ccab2896f49, data reload: false

query1	0.01	0.01	0.00
query2	0.09	0.05	0.05
query3	0.26	0.14	0.14
query4	1.60	0.13	0.13
query5	0.23	0.21	0.22
query6	1.16	0.80	0.81
query7	0.05	0.01	0.00
query8	0.06	0.03	0.04
query9	0.37	0.30	0.30
query10	0.54	0.55	0.54
query11	0.19	0.13	0.14
query12	0.18	0.15	0.14
query13	0.46	0.46	0.46
query14	0.99	1.00	1.00
query15	0.62	0.60	0.59
query16	0.32	0.32	0.32
query17	1.11	1.07	1.08
query18	0.22	0.21	0.21
query19	2.10	2.05	1.99
query20	0.01	0.02	0.01
query21	15.44	0.22	0.14
query22	4.89	0.05	0.05
query23	16.08	0.30	0.12
query24	3.01	0.40	0.31
query25	0.11	0.04	0.04
query26	0.73	0.21	0.16
query27	0.04	0.04	0.04
query28	3.44	0.82	0.36
query29	12.48	4.01	3.19
query30	0.28	0.15	0.15
query31	2.77	0.57	0.32
query32	3.22	0.60	0.50
query33	3.22	3.17	3.15
query34	15.70	4.00	3.32
query35	3.24	3.27	3.24
query36	0.56	0.44	0.44
query37	0.09	0.07	0.06
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.18	0.15	0.14
query41	0.09	0.03	0.03
query42	0.03	0.03	0.03
query43	0.05	0.04	0.03
Total cold run time: 96.31 s
Total hot run time: 24.01 s

[hello-stephen]

FE UT Coverage Report

Increment line coverage 82.26% (153/186) 🎉
Increment coverage report
Complete coverage report

[hello-stephen]

FE Regression Coverage Report

Increment line coverage 59.68% (111/186) 🎉
Increment coverage report
Complete coverage report

[starocean999]

/review

[github-actions]

Review result

Requesting changes for two distinct P1 planner-correctness defects at head cc467bf83a8fcd2d081b58b1c68c1ccab2896f49: a bounded simplifier retry can report success without producing the full bitmap, and a nullable-side forwarding alias can be reconstructed with stale non-nullable metadata.

Critical checkpoints:

• Goal and proof: The patch is intended to preserve nullable-side alias semantics through DPHyp reordering. It does not fully accomplish that goal because the two inline cases remain reachable. The changed tests do not force the low-dphyper_limit simplifier/retry path or select a direct forwarding alias on unmatched LEFT/RIGHT/FULL rows while checking output nullability.
• Scope and clarity: The nine changed files are focused on HyperGraph alias construction, receiver parity, and their tests. The risks arise at the intended integration boundaries rather than from unrelated changes.
• Concurrency: HyperGraph construction and enumeration are synchronous, query-local operations. No new thread, shared mutable state, lock, or deadlock risk was introduced.
• Lifecycle: Builder layer state is saved/restored recursively and both receivers reset per enumeration retry. No resource ownership, static initialization, or abnormal cleanup lifecycle applies.
• Configuration: No configuration item is added. The existing dphyper_limit is material to the first defect and needs a regression that reaches its bounded retry path.
• Compatibility: No public symbol, persisted/storage format, rolling-upgrade contract, or cross-version protocol changes.
• Parallel paths: Counter versus PlanReceiver, initial enumeration versus GraphSimplifier retry, missed/enforced/conflict edges, and LEFT/RIGHT/FULL nullable-child paths were traced. The two inline failures are distinct from the already-live comments on invalid cross-bitmap Project flattening and RIGHT/FULL complex-alias coverage.
• Conditional and error paths: The new alias-dependency gates return CONTINUE. That rejection is locally intentional, but enumeration completion is incorrectly treated as feasible success when no full-bitmap group exists. No ignored Status or swallowed exception applies in this FE Java path.
• Test coverage: Unit tests mostly prove only that some plan exists and can accept original-group fallback. Regression coverage has deterministic ordered queries, but only LEFT-join execution; it lacks the two targeted oracles above. Existing live threads already cover the other fallback-tolerant, raw-slot, boundary, dependency-order, and RIGHT/FULL gaps, so they are not duplicated here.
• Test results: The ten changed regression query blocks match the ten output blocks and their expected values are internally consistent. Current remote compile, FE UT, CheckStyle, P0, nonConcurrent, and coverage checks passed; this review was static-only as required, so no local build or test was run.
• Observability: Existing planner tracing is sufficient for this local rewrite; no distinct logging or metric gap was found.
• Transactions, persistence, and writes: No transaction, EditLog, data-write, crash-recovery, memory-safety, or storage-visibility path is changed.
• FE/BE propagation: No new variable or protocol field needs propagation between FE and BE.
• Performance: The added edge/alias scans are bounded planner work. No distinct performance defect beyond already-reviewed raw-slot retention concerns was substantiated.
• Other issues: The older mixed-join-type Counter/PlanReceiver asymmetry was rechecked but predates this PR, and no new reachable failure attributable to this patch was established.
• User focus: The supplied focus file contains no additional review focus; the entire PR was reviewed.
• Completion: Two review rounds converged. Both normal reviewers and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS against this frozen two-comment set after a full existing-thread deduplication and current-head anchor check.

[starocean999]

/review

[starocean999]

run buildall