github-actions[bot] commented on code in PR #65682:
URL: https://github.com/apache/doris/pull/65682#discussion_r3600338217
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/HyperGraph.java:
##########
@@ -385,15 +478,54 @@ 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);
+ // 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.
+ // Exception: a bare Slot-forwarding alias (e.g. s = A.k)
substitutes
+ // values without changing evaluation count; mapping to
subTreeNodes would
+ // block beneficial {A,C} joins before B. Keep those on the
underlying
+ // slot's bitmap — the emission boundary is still enforced by
+ // nodeToProjectedAliases keyed by subTreeNodes.
+ long mapNodes;
+ if (isNullableSide && alias.child() instanceof Slot) {
Review Comment:
[P1] Substitute the forwarding alias before exposing its endpoint
In the reduced nullable subtree below, the retained complex co-alias
prevents the Project from being pushed through `A JOIN B`, while this exception
lets the `s = C.k` edge use `{A}--{C}`:
```text
X LEFT JOIN
X
InnerJoin(s = C.k)
Project(A.k AS s, A.v + B.v AS keep) // keep is consumed above
InnerJoin(A.k = B.k)
A
B
C
```
The nullable branch returns before adding `s -> A.k` to `aliasReplaceMap`,
so `addJoin` leaves the actual predicate as `s = C.k`. If DPHyp enumerates
`{A,C}` first, neither child outputs `s`; its Project layer remains keyed by
`{A,B}`, and the rewritten join violates the child-output invariant. This is a
correctness follow-up to the existing minimal-endpoint thread: the endpoint
optimization landed without the safe substitution it assumes. Rewrite
forwarding consumers to the underlying slot while retaining the separate output
boundary, or keep the full source endpoint, and test this normalization-stable
shape.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/HyperGraph.java:
##########
@@ -125,32 +129,82 @@ public Edge getJoinEdge(int index) {
return joinEdges.get(index);
}
- public boolean hasLiteralAlias() {
- return !nodeToLiteralAlias.isEmpty();
+ public boolean hasProjectedAliases() {
+ return !nodeToProjectedAliases.isEmpty();
}
/**
* find all literal alias should be projected after left join right
*/
- public List<NamedExpression> getLiteralAlias(long left, long right) {
+ public List<NamedExpression> getProjectedAliases(long left, long right) {
ImmutableList.Builder<NamedExpression> aliasList =
ImmutableList.builder();
if (left == right) {
- List<NamedExpression> namedExpressions =
nodeToLiteralAlias.get(left);
- if (namedExpressions != null) {
- aliasList.addAll(namedExpressions);
+ List<List<NamedExpression>> layers =
nodeToProjectedAliases.get(left);
+ if (layers != null) {
+ layers.forEach(aliasList::addAll);
}
} else {
long nodes = LongBitmap.newBitmapUnion(left, right);
- for (Map.Entry<Long, List<NamedExpression>> entry :
nodeToLiteralAlias.entrySet()) {
+ for (Map.Entry<Long, List<List<NamedExpression>>> entry :
nodeToProjectedAliases.entrySet()) {
if (!LongBitmap.isSubset(entry.getKey(), left) &&
!LongBitmap.isSubset(entry.getKey(), right)
&& LongBitmap.isSubset(entry.getKey(), nodes)) {
- aliasList.addAll(entry.getValue());
+ entry.getValue().forEach(aliasList::addAll);
}
}
}
return aliasList.build();
}
+ /**
+ * Returns ordered Project layers for the given left/right nodes.
+ * Each inner list is one LogicalProject layer; layers are ordered
bottom-up.
+ * PlanReceiver emits each layer as a separate LogicalProject to preserve
+ * materialization boundaries for volatile expressions.
+ */
+ public List<List<NamedExpression>> getProjectedAliasLayers(long left, long
right) {
+ List<List<NamedExpression>> result = new ArrayList<>();
+ if (left == right) {
+ List<List<NamedExpression>> layers =
nodeToProjectedAliases.get(left);
+ if (layers != null) {
+ result.addAll(layers);
+ }
+ } else {
+ long nodes = LongBitmap.newBitmapUnion(left, right);
+ for (Map.Entry<Long, List<List<NamedExpression>>> entry :
nodeToProjectedAliases.entrySet()) {
+ if (!LongBitmap.isSubset(entry.getKey(), left) &&
!LongBitmap.isSubset(entry.getKey(), right)
Review Comment:
[P1] Keep preserved layers at their exact source boundary
This predicate also accepts a source key that first becomes available only
in a larger reordered tree. For example:
```text
X LEFT JOIN
X
InnerJoin(A.k = C.k)
Project(A.k, concat(B.v, uuid()) AS u)
InnerJoin(A.k = B.k)
A
B
C
```
If DPHyp builds `{A,C}` first, the later `{A,C}` + `{B}` join satisfies
these checks for key `{A,B}`. `PlanReceiver` therefore builds `{A,B,C}` and
only then emits `u`; originally `u` is evaluated after `A-B` and before `C`. A
multiplying/filtering `C` changes volatile evaluation count and can suppress a
non-movable error. This is distinct from the existing downward-placement
thread: full-subtree keying prevents emission below `{A,B}`, but eligibility
still permits emission above it. Treat source keys as exact materialization
barriers (or reject alternatives adding outside nodes first), and add a
reordered volatile/non-movable regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/receiver/PlanReceiver.java:
##########
@@ -291,29 +291,62 @@ public Group getBestPlan(long bitmap) {
private LogicalPlan proposeProject(LogicalPlan join, List<Edge> edges,
long left, long right) {
Set<Slot> outputSet = join.getOutputSet();
- // calculate required columns by all parents
- Set<Slot> requireSlots = calculateRequiredSlots(left, right, edges);
+ // calculate required columns by all parents (final outputs + unused
edges)
+ Set<Slot> parentRequireSlots = calculateRequiredSlots(left, right,
edges);
+ // Pending projected aliases may reference input slots (e.g., A.v, B.v
for
+ // s=A.v+B.v) that are not in finalRequiredSlots or unused edges.
Preserve
+ // them so the join output still contains the base columns needed to
evaluate
+ // the alias expressions, both for aliases emitted at this stage and
those
+ // deferred to a later join whose bitmap is a superset.
+ Set<Slot> aliasInputSlots = hyperGraph.getAllAliasInputSlotsForNodes(
+ LongBitmap.newBitmapUnion(left, right));
+ Set<Slot> requireSlots = new HashSet<>(parentRequireSlots);
+ requireSlots.addAll(aliasInputSlots);
List<NamedExpression> allProjects = new ArrayList<>(outputSet.size());
for (Slot slot : outputSet) {
if (requireSlots.contains(slot)) {
allProjects.add(slot);
}
}
- if (hyperGraph.hasLiteralAlias()) {
- allProjects.addAll(hyperGraph.getLiteralAlias(left, right));
- }
if (allProjects.isEmpty()) {
allProjects.add(new Alias(new ExprId(-1), new
TinyIntLiteral((byte) 1)));
}
- // propose logical project
+ // propose logical project for the slot pass-through
LogicalPlan logicalPlan;
if (outputSet.equals(new HashSet<>(allProjects))) {
logicalPlan = join;
} else {
logicalPlan = new LogicalProject<>(allProjects, join);
}
+
+ // Emit projected alias layers as separate LogicalProject nodes.
+ // Each layer corresponds to one original Project in the source plan,
+ // preserving materialization boundaries for volatile expressions
+ // (e.g., uuid()) that cannot be flattened into a single Project.
+ // Carry forward child slots still required by parents (e.g., join
keys)
+ // that the layer's aliases do not produce, so the parent plan can
still
+ // reference them. Use parentRequireSlots (not requireSlots) so that
raw
+ // alias inputs like B.payload are dropped after the alias
materializes,
+ // matching the original Project's output boundary.
+ if (hyperGraph.hasProjectedAliases()) {
+ for (List<NamedExpression> layer :
hyperGraph.getProjectedAliasLayers(left, right)) {
+ List<NamedExpression> mergedLayer = new ArrayList<>(layer);
+ Set<ExprId> layerExprIds = new HashSet<>();
+ for (NamedExpression a : layer) {
+ layerExprIds.add(a.getExprId());
+ }
+ for (Slot childSlot : logicalPlan.getOutputSet()) {
+ if (parentRequireSlots.contains(childSlot)
Review Comment:
[P1] Preserve inputs until the last Project layer consumes them
In the nullable child of an ancestor outer join, a join-separated later
layer can need a raw slot that is not a final output or unused-edge input:
```text
X LEFT JOIN
X
Project[x, B.w + 1 AS y]
InnerJoin(A.k = B.k)
A
Project[B.k, B.w, B.v + 1 AS x]
B
```
The join prevents these Projects from being normalized into one layer. At
leaf B, `getAllAliasInputSlotsForNodes` initially keeps `B.v` and `B.w`, while
`parentRequireSlots` keeps join key `B.k` but not `B.w`. The lower layer
becomes `Project[x, B.k]` and drops `B.w`; after `A JOIN B`, the upper layer
still evaluates `y = B.w + 1` against a child lacking `B.w`. This is distinct
from the existing parent join-key thread because `B.w` is required only by a
later Project layer. Carry inputs of all remaining layers until their last
consumer, then drop them, and test this join-separated nullable-side case.
--
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]
