github-actions[bot] commented on code in PR #65682:
URL: https://github.com/apache/doris/pull/65682#discussion_r3700925486
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/HyperGraph.java:
##########
@@ -385,15 +602,57 @@ 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);
Review Comment:
[P1] Preserve nullability for forwarding aliases
The reduced tree is:
```text
Project(s#S)
LeftOuterJoin(X.k = s#S)
X
Project(B.k NOT NULL AS s#S)
B
```
This condition excludes `s` from the retained nullable-side layer, so
`aliasReplaceMap` stores `s -> B.k` and the final Project is rebuilt as
`Alias(#S, B.k)` above the outer join. The join does produce a nullable `B.k`,
but that Alias still holds the original non-nullable Slot object;
`Alias.toSlot()` therefore advertises `s` as non-nullable. Slot equality and
`CheckAfterRewrite` use only ExprId, so the mismatch passes validation, and
`PlanTranslatorContext` copies the false flag into the physical
`SlotDescriptor` even though unmatched rows produce NULL.
Please keep output materialization separate from endpoint
substitution—retain this alias below null-extension or rebuild it from the
actual post-join Slot—and add a forced DPHyp test that selects the forwarding
alias for unmatched LEFT/RIGHT/FULL rows and checks output nullability.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/receiver/Counter.java:
##########
@@ -54,13 +61,34 @@ public Counter(int limit) {
public EmitState emitCsgCmp(long left, long right, List<Edge> edges) {
Preconditions.checkArgument(counter.containsKey(left));
Preconditions.checkArgument(counter.containsKey(right));
- if (!checkConflictRule(left, right, edges)) {
+ // Mirror PlanReceiver.emitCsgCmp: find missed edges first, reject the
+ // pair when an enforced-order / unsafe alias edge is found, then count
+ // the pair before the conflict-rule and alias-dependency checks (same
+ // ordering as PlanReceiver, so GraphSimplifier's limit decision
matches
+ // what PlanReceiver actually emits).
+ List<Edge> missingEdges = new ArrayList<>();
+ if (!processMissedEdges(hyperGraph, usdEdges, left, right, edges,
missingEdges)) {
return EmitState.CONTINUE;
}
emitCount += 1;
if (emitCount > limit) {
return EmitState.FAIL;
}
+ edges.addAll(missingEdges);
+ if (!checkConflictRule(left, right, edges)) {
+ return EmitState.CONTINUE;
+ }
+ // Reject cross-bitmap alias layer dependencies, same as PlanReceiver.
+ if (hyperGraph.hasUnresolvableAliasDependency(left, right)) {
Review Comment:
[P1] Require enumeration to produce the full bitmap
A reduced alias chain is:
```text
Project(y#Y = x#X + C.v) // key {A,B,C}
Join(A.k = C.k) // edge A-C
Project(x#X = A.v + B.v) // key {A,B}
Join(A.k = B.k) // edge A-B
```
With a bounded `dphyper_limit`, `GraphSimplifier` can cost-order A-C before
A-B; `concretizeSimplificationStep` then extends A-B to `{A,C}--{B}`. At that
only full split this new branch returns `CONTINUE`, so Counter never inserts
`{A,B,C}`. However `SubgraphEnumerator.enumerate()` still returns true because
no receiver returned `FAIL`; the simplifier accepts that rootless probe, and
the final PlanReceiver pass repeats the rejection. `JoinOrderJob` then returns
`getBestPlan(fullBitmap)`, which is null instead of the original-group fallback.
Please make enumeration success require
`receiver.contain(hyperGraph.getNodesMap())` (or an equivalent feasible-root
signal) before the simplifier/final retry accepts it, and add a low-limit
regression that forces this producer-after-consumer order.
--
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]
