github-actions[bot] commented on code in PR #65682:
URL: https://github.com/apache/doris/pull/65682#discussion_r3679494275
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/receiver/PlanReceiver.java:
##########
@@ -291,29 +300,78 @@ 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 aliases as a single LogicalProject node.
+ // Cross-layer references (e.g., z = x + 1 referencing x = COALESCE(v,
0))
+ // were already resolved at graph-build time, so only one Project is
needed.
+ // Carry forward child slots still required by parents (e.g., join
keys)
+ // or by deferred alias layers (e.g., B.w for a later y=B.w+1).
+ // Use the full requireSlots so that deferred-layer inputs survive
+ // through intermediate layers.
+ if (hyperGraph.hasProjectedAliases()) {
+ List<NamedExpression> aliases =
hyperGraph.getProjectedAliases(left, right);
Review Comment:
[P1] Keep cross-bitmap dependent alias layers ordered
A reduced nullable subtree is:
```text
X LEFT JOIN
X
Project(y#Y = x#X + C.v)
InnerJoin(A.k = C.k)
Project(x#X = A.v + B.v)
InnerJoin(A.k = B.k)
A
B
C
```
DPHyp can build `{A,C}` first and then join `{B}`. At that final split,
`getProjectedAliases({A,C},{B})` returns both `x` (key `{A,B}`) and `y` (key
`{A,B,C}`), and this method constructs one `LogicalProject([x=A.v+B.v, y=x+C.v,
...], child)`. Project expressions read their child, not sibling outputs, so
the child has no ExprId `#X` for `y`; `CheckAfterRewrite` rejects the selected
alternative.
The same-bitmap replacement does not cover these different keys. Please emit
the bitmap/source layers in dependency order (or reject the superset
alternative until the producer layer is in one child), and add a test that
forces `{A,C}+{B}`, checks slot validity, and fails on original-group fallback.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/HyperGraph.java:
##########
@@ -330,23 +423,75 @@ public void updateNode(int idx, Group group) {
* latest join edges index
*/
private Pair<BitSet, Long> buildForDPhyper(GroupExpression
groupExpression) {
+ return buildForDPhyper(groupExpression, false);
+ }
+
+ private Pair<BitSet, Long> buildForDPhyper(GroupExpression
groupExpression, boolean isNullableSide) {
// process Project
if (isValidProject(groupExpression.getPlan())) {
LogicalProject<?> project = (LogicalProject<?>)
groupExpression.getPlan();
- Pair<BitSet, Long> res =
buildForDPhyper(groupExpression.child(0).getLogicalExpressions().get(0));
+ Pair<BitSet, Long> res = buildForDPhyper(
+
groupExpression.child(0).getLogicalExpressions().get(0), isNullableSide);
+ // Start a new layer for this Project. Each source Project
becomes one
+ // LogicalProject layer, preserving materialization boundaries
for
+ // volatile expressions (e.g., uuid()) that
PlanUtils.canMergeWithProjections
+ // would otherwise reject.
+ List<NamedExpression> savedLayer =
this.currentProjectedAliasLayer;
+ this.currentProjectedAliasLayer = new ArrayList<>();
for (NamedExpression expr : project.getProjects()) {
if (expr instanceof Alias) {
- this.addAlias((Alias) expr, res.second);
+ this.addAlias((Alias) expr, res.second,
isNullableSide);
}
}
+ // Flush the layer if non-empty. If aliases for this key
already
+ // exist, resolve cross-layer references (e.g., z = x + 1 where
+ // x was defined by an earlier Project on the same subtree) and
+ // merge into the existing single layer. Cross-layer resolution
+ // is safe because volatile/non-movable expressions are already
+ // excluded by isValidProject.
+ if (!this.currentProjectedAliasLayer.isEmpty()) {
+ long key = res.second;
+ List<NamedExpression> existing =
nodeToProjectedAliases.get(key);
+ if (existing != null) {
+ Map<Slot, Expression> replaceMap = new
LinkedHashMap<>();
+ for (NamedExpression a : existing) {
+ if (a instanceof Alias) {
+ replaceMap.put(a.toSlot(), ((Alias)
a).child());
+ }
+ }
+ for (NamedExpression expr :
currentProjectedAliasLayer) {
+ existing.add((NamedExpression)
ExpressionUtils.replace(expr, replaceMap));
+ }
+ } else {
+ nodeToProjectedAliases.put(key,
+ new ArrayList<>(currentProjectedAliasLayer));
+ }
+ }
+ this.currentProjectedAliasLayer = savedLayer;
return res;
}
// process Join
if (isValidJoin(groupExpression.getPlan())) {
LogicalJoin<?, ?> join = (LogicalJoin<?, ?>)
groupExpression.getPlan();
- Pair<BitSet, Long> left =
buildForDPhyper(groupExpression.child(0).getLogicalExpressions().get(0));
- Pair<BitSet, Long> right =
buildForDPhyper(groupExpression.child(1).getLogicalExpressions().get(0));
+ JoinType joinType = join.getJoinType();
+ // Determine if children are on the nullable side:
+ // - For LEFT OUTER JOIN, the right child is nullable
+ // - For RIGHT OUTER JOIN, the left child is nullable
+ // - For FULL OUTER JOIN, both children are nullable
+ // - If we're already inside a nullable context, propagate down
+ boolean leftNullable = isNullableSide
Review Comment:
[P2] Cover the new RIGHT/FULL nullable-side branches
These conditionals add distinct behavior for a nullable left child (`RIGHT
OUTER JOIN`) and for both nullable children (`FULL OUTER JOIN`), but every
changed FE and regression query exercises only `LEFT JOIN`. A reversed child
flag or a one-sided FULL implementation would therefore pass the new suite, and
the generic outer-join tests do not exercise this DPHyp alias reconstruction.
Please add a RIGHT case with a consumed complex alias on the left and
unmatched rows, plus a FULL case with consumed aliases on both inputs. Force or
inspect the DPHyp result so original-group fallback cannot satisfy the test,
and assert the exact NULL/slot-nullability behavior.
--
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]
