morrySnow commented on code in PR #65682:
URL: https://github.com/apache/doris/pull/65682#discussion_r3671912125
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/HyperGraph.java:
##########
@@ -330,23 +404,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));
Review Comment:
[P1] Preserve the expression-limit fallback
This unconditional replacement flattens every same-bitmap Project chain, but
normal Project merging deliberately uses `PlanUtils.tryMergeProjections` and
keeps layers separate when expansion hits `EXPRESSION_EXCEEDS_LIMIT`. In a
nullable-side chain `x1=B.v+B.v`, `x2=x1+x1`, ... the original/rewrite plan
remains valid once the configured width/depth limit prevents further merging;
DPHyp graph construction expands the chain again here and `Expression`
construction throws, so enabling DPHyp turns a valid query into an analysis
failure. Preserve ordered layers, or catch the limit and retain the
materialization boundary, and add a limit-path regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraphv2/receiver/PlanReceiver.java:
##########
@@ -291,29 +300,61 @@ 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);
Review Comment:
[P2] Stop carrying alias inputs after materialization
`getAllAliasInputSlotsForNodes` returns inputs for every overlapping alias
even when that alias bitmap is already wholly contained in a completed child,
and this set is copied into each rebuilt Project. For `Project[B.k,
expensive(B.payload) AS dv]`, the leaf becomes `[dv, B.k, B.payload]`; every
ancestor candidate still overlaps `{B}`, so DPHyp costs physical alternatives
with the wide payload although the original Project dropped it. Since
distribution and join costs use child output size, this can distort join-order
selection and grow planning state (later ColumnPruning only removes the slot
after DPHyp has chosen an alternative). Track inputs only until the alias is
emitted, then retain just the alias and true parent/deferred-layer
requirements; the current test also needs a plan-shape/output assertion because
`assertNotNull` cannot catch this.
--
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]
