github-actions[bot] commented on code in PR #65682:
URL: https://github.com/apache/doris/pull/65682#discussion_r3592512010
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/HyperGraph.java:
##########
@@ -385,17 +406,33 @@ public boolean addAlias(Alias alias, long subTreeNodes) {
if (bitmap == 0) {
bitmap = subTreeNodes;
addToReplaceMap = false;
- List<NamedExpression> aliasList =
nodeToLiteralAlias.get(bitmap);
+ List<NamedExpression> aliasList =
nodeToProjectedAliases.get(bitmap);
if (aliasList == null) {
aliasList = new ArrayList<>(1);
- nodeToLiteralAlias.put(bitmap, aliasList);
+ nodeToProjectedAliases.put(bitmap, aliasList);
}
aliasList.add(alias);
}
Preconditions.checkArgument(bitmap > 0, "slot must belong to some
table");
slotToHyperNodeMap.put(aliasSlot, 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 nodeToProjectedAliases so they get
projected at the leaf
+ // level, preserving the original execution boundary below the
outer join.
+ if (addToReplaceMap && isNullableSide) {
Review Comment:
[P1] Preserve the original Project boundary
This map is keyed by the alias's referenced bitmap, so a Project above a
nested join can be rebuilt below it. For example:
```text
X LEFT JOIN
X
Project(A.k, coalesce(B.v,0) AS dv)
A LEFT JOIN B
```
The ancestor marks the right subtree nullable, but `addAlias` records `dv`
under `{B}`. `addGroup({B})` therefore evaluates it on B; when B is empty, the
inner LEFT JOIN null-extends `dv` to NULL. In the original tree the inner join
first supplies `B.v=NULL`, then the Project returns 0, so a matching X/A row
changes from 0 to NULL. The same placement also moves
`NoneMovableFunction`/volatile aliases across inner-join filtering or row
multiplication. Please retain the source Project's subtree/stage rather than
emitting at the smallest referenced bitmap, and cover nested outer and
non-movable cases.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/HyperGraph.java:
##########
@@ -385,17 +406,33 @@ public boolean addAlias(Alias alias, long subTreeNodes) {
if (bitmap == 0) {
bitmap = subTreeNodes;
addToReplaceMap = false;
- List<NamedExpression> aliasList =
nodeToLiteralAlias.get(bitmap);
+ List<NamedExpression> aliasList =
nodeToProjectedAliases.get(bitmap);
if (aliasList == null) {
aliasList = new ArrayList<>(1);
- nodeToLiteralAlias.put(bitmap, aliasList);
+ nodeToProjectedAliases.put(bitmap, aliasList);
}
aliasList.add(alias);
}
Preconditions.checkArgument(bitmap > 0, "slot must belong to some
table");
slotToHyperNodeMap.put(aliasSlot, 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 nodeToProjectedAliases so they get
projected at the leaf
+ // level, preserving the original execution boundary below the
outer join.
+ if (addToReplaceMap && isNullableSide) {
+ List<NamedExpression> aliasList =
nodeToProjectedAliases.get(bitmap);
+ if (aliasList == null) {
+ aliasList = new ArrayList<>(1);
+ nodeToProjectedAliases.put(bitmap, aliasList);
+ }
+ aliasList.add(alias);
Review Comment:
[P1] Keep dependent aliases in separate projection layers
`aliasList.add(alias)` happens before replacement, and aliases on this path
never enter `aliasReplaceMap`. A reachable nullable subtree can contain
`Project(y=x+1)` above a join whose B child defines `Project(coalesce(B.v,0) AS
x)`. Both aliases map to `{B}`, so `addGroup({B})` builds one
`LogicalProject([x=..., y=x+1], B)`. Project expressions read their child, not
sibling outputs; B does not output ExprId x, and `CheckAfterRewrite` rejects y.
Preserve ordered Project layers or safely expand dependencies before coalescing
same-bitmap aliases; add a chained-alias regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/receiver/PlanReceiver.java:
##########
@@ -299,8 +299,8 @@ private LogicalPlan proposeProject(LogicalPlan join,
List<Edge> edges, long left
allProjects.add(slot);
}
}
- if (hyperGraph.hasLiteralAlias()) {
- allProjects.addAll(hyperGraph.getLiteralAlias(left, right));
+ if (hyperGraph.hasProjectedAliases()) {
+ allProjects.addAll(hyperGraph.getProjectedAliases(left, right));
Review Comment:
[P1] Retain pending alias inputs until their bitmap is built
For a nullable child `Project(A.v+B.v AS s) over (A JOIN B on keys)`, this
entry is withheld until `{A,B}`. `calculateRequiredSlots` sees final slot s and
unused edge keys, but not A.v/B.v, so leaf projects prune those values. When
line 303 finally appends s, the rebuilt join no longer outputs either input,
and `CheckAfterRewrite` fails. The legacy receiver explicitly adds input slots
of pending complex projects. Expose pending aliases to required-slot
calculation (including transitive inputs) until emission, and add a multi-node
alias 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]
