github-actions[bot] commented on code in PR #65682:
URL: https://github.com/apache/doris/pull/65682#discussion_r3612940972
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/HyperGraph.java:
##########
@@ -385,15 +528,46 @@ public boolean addAlias(Alias alias, long subTreeNodes) {
if (bitmap == 0) {
bitmap = subTreeNodes;
addToReplaceMap = false;
- List<NamedExpression> aliasList =
nodeToLiteralAlias.get(bitmap);
- if (aliasList == null) {
- aliasList = new ArrayList<>(1);
- nodeToLiteralAlias.put(bitmap, aliasList);
+ // Constant aliases go into the current Project layer (set up
by
+ // buildForDPhyper) and will be flushed to
nodeToProjectedAliases
+ // keyed by the layer's subtree bitmap after the Project is
processed.
+ if (currentProjectedAliasLayer != null) {
+ currentProjectedAliasLayer.add(alias);
}
- aliasList.add(alias);
}
Preconditions.checkArgument(bitmap > 0, "slot must belong to some
table");
- slotToHyperNodeMap.put(aliasSlot, bitmap);
+ boolean mustStayInCurrentAliasLayer = isNullableSide &&
!(alias.child() instanceof Slot);
+ // Map nullable-side alias slots to subTreeNodes instead of the
minimal
+ // referenced bitmap. Otherwise a later join predicate like s=C.k
sees s as
+ // {B} (its input slot) and creates a {B}--{C} edge, allowing
DPHyp to join
+ // B and C before A — but s can only be emitted when {A,B} is
complete.
+ // Using subTreeNodes (e.g. {A,B}) forces the predicate edge to
require the
+ // full source subtree, matching the emission key in
nodeToProjectedAliases.
+ // Note: always use subTreeNodes for nullable-side aliases. A
Slot-forwarding
+ // alias (e.g. s=A.k) that shares a Project with expression
aliases cannot
+ // safely use the minimal bitmap — its layer only emits at {A,B},
so exposing
+ // it as {A} would let DPHyp form predicate edges before the alias
exists.
+ slotToHyperNodeMap.put(aliasSlot, mustStayInCurrentAliasLayer ?
subTreeNodes : bitmap);
+ // Do not add aliases on the nullable side of outer joins to
aliasReplaceMap.
+ // Aliases on the nullable side (e.g., COALESCE(v, 0) AS dv on the
right side of
+ // a LEFT JOIN) must execute BEFORE the outer join's
null-extension.
+ // If added to aliasReplaceMap, they would be unwrapped and
reconstructed above
+ // the outer join by PlanReceiver.proposeProject(), changing
execution order
+ // and producing wrong results.
+ // Instead, add them to the current Project layer. buildForDPhyper
flushes each
+ // layer to nodeToProjectedAliases keyed by subTreeNodes so the
alias is only
+ // projected when the full original subtree is available,
preserving the
+ // execution boundary and volatile materialization order.
+ // No replaceNameExpression here: nullable-side aliases are stored
as
+ // independent layers that reference child-output slots, so
expansion is
+ // unnecessary and could trigger expression-limit failures for
large chains
+ // (the same reason PlanUtils.tryMergeProjections keeps layers).
+ if (addToReplaceMap && mustStayInCurrentAliasLayer) {
+ if (currentProjectedAliasLayer != null) {
+ currentProjectedAliasLayer.add(alias);
Review Comment:
[P1] Resolve forwarding aliases before storing a later layer
A join-separated chain can reach this branch:
```text
X LEFT JOIN
X
Project(A.k, x + 1 AS y)
InnerJoin(A.k = B.k)
Project(A.k, A.v AS x)
A
B
```
On the nullable traversal, the lower `x = A.v` has a direct `Slot` child, so
it is removed into `aliasReplaceMap` and no layer ever emits ExprId `x`. The
upper `y = x + 1` enters this branch, but the early return stores the original
expression before `replaceNameExpression` runs. `getAllAliasInputSlotsForNodes`
therefore asks the leaf to preserve `x`, not `A.v`; no rebuilt child outputs
`x`, and `PlanReceiver` eventually creates `Project(y=x+1)` over a child that
lacks that slot, which `CheckAfterRewrite` rejects.
This is distinct from the earlier join-predicate forwarding thread:
`addJoin` now substitutes forwarding aliases, but preserved Project bodies do
not. Please apply the safe forwarding map before storing this layer (or retain
the forwarding producer as a real lower layer), and add this join-separated
chain as a regression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
