github-actions[bot] commented on code in PR #69388:
URL: https://github.com/apache/doris/pull/69388#discussion_r4334911111


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java:
##########
@@ -1064,10 +1066,10 @@ private LogicalPlan 
bindUsingJoin(MatchingContext<LogicalUsingJoin<Plan, Plan>>
                 mergeExpr = new Alias(new Coalesce(leftSlot, rightSlot), 
colName);
             } else if (joinType.isRightJoin() || 
joinType.isRightSemiOrAntiJoin()) {
                 // RIGHT OUTER / RIGHT SEMI / RIGHT ANTI: use right side 
(preserved side)
-                mergeExpr = new Alias(rightSlot, colName);
+                mergeExpr = rightSlot.withQualifier(ImmutableList.of());
             } else {
                 // LEFT OUTER / INNER / LEFT SEMI / LEFT ANTI / CROSS: use 
left side
-                mergeExpr = new Alias(leftSlot, colName);
+                mergeExpr = leftSlot.withQualifier(ImmutableList.of());

Review Comment:
   [P1] Preserve CTE column order when sharing the USING-key ExprId
   
   `WITH q(x,y,z) AS (SELECT a,t1.b,t1.a FROM t1 JOIN t2 USING(a)) SELECT * 
FROM q` now exposes its columns in the wrong order. The producer becomes:
   
   ```text
   Project(a#L, t1.b#B, t1.a#L)
     Join(t1.a = t2.a)
   ```
   
   `AnalyzeCTE` wraps this with column names `[x#L,y#B,z#L]`, retaining the 
ExprIds. `LogicalCTEConsumer` then keys `producerToConsumerOutputMap` by 
producer Slot, whose equality compares only ExprId, and `computeOutput()` 
returns that multimap's grouped `values()`: `[x,z,y]`. Star expansion binds 
this order before CTE inlining, which does not repair it. With a matching 
`t1(a,b)=(1,10)` row, the result is `(1,1,10)` instead of `(1,10,1)`; 
positional consumers such as UNION or INSERT SELECT can consequently receive 
wrong values. Previously the merged Alias had a separate ExprId, so this query 
preserved its column order. The right-preserved branch has the same issue.
   
   Preserve producer column positions in the CTE consumer alongside this 
identity change, and add a result-checking regression with merged and qualified 
keys separated by a non-key column.



-- 
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]

Reply via email to