brucearctor commented on code in PR #69938:
URL: https://github.com/apache/doris/pull/69938#discussion_r4446644702


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownProject.java:
##########
@@ -338,6 +342,53 @@ private static List<NamedExpression> replaceSlot(
         return pushedOutput;
     }
 
+    /**
+     * Returns true if {@code expression} references any {@link ArrayItemSlot} 
that is not bound
+     * by an enclosing {@link Lambda} within {@code expression} itself.
+     *
+     * <p>{@link Expression#getInputSlots()} deliberately excludes {@link 
ArrayItemSlot}, so a
+     * sub-expression inside a lambda body (such as {@code element_at(p, 1)}) 
has an empty
+     * {@code getInputSlots()} set when it only depends on the lambda 
parameter {@code p}.
+     * Pushing such a sub-expression out of its enclosing {@link Lambda} into 
a child
+     * {@link LogicalProject} leaves {@code p} unbound outside {@code 
visitArrayMap}, causing
+     * {@code ExpressionTranslator.visitArrayItemSlot} to return {@code null}.
+     */
+    @VisibleForTesting
+    static boolean hasFreeLambdaArgument(Expression expression) {
+        return hasFreeLambdaArgument(expression, ImmutableSet.of());
+    }
+
+    private static boolean hasFreeLambdaArgument(Expression expression, 
Set<ExprId> boundLambdaArgs) {
+        if (!expression.containsType(ArrayItemSlot.class)) {
+            return false;
+        }
+        if (expression instanceof ArrayItemSlot) {
+            return !boundLambdaArgs.contains(((ArrayItemSlot) 
expression).getExprId());
+        }
+        if (expression instanceof Lambda) {
+            Lambda lambda = (Lambda) expression;
+            // The array expression iterated by each lambda argument is 
evaluated in the outer scope,
+            // before this lambda's own parameters are bound.
+            for (ArrayItemReference arg : lambda.getLambdaArguments()) {
+                if (hasFreeLambdaArgument(arg.getArrayExpression(), 
boundLambdaArgs)) {
+                    return true;
+                }
+            }
+            ImmutableSet.Builder<ExprId> nextBoundArgs = 
ImmutableSet.<ExprId>builder()
+                    .addAll(boundLambdaArgs);

Review Comment:
   Addressed in be7eac6c:
   1. Check `child.getOutputSet().containsAll(e.getInputSlots())` before 
calling `hasFreeLambdaArgument(e)` (and `break` out of the child loop if 
`hasFreeLambdaArgument(e)` is true), so cross-child candidates skip the lambda 
scope scan entirely.
   2. Track bound lambda parameter `ExprId`s with a single mutable 
`HashSet<ExprId>` using scoped add/remove (`try ... finally`) instead of 
copying `ImmutableSet` at each nested `Lambda`.
   3. Added a deeply nested lambda chain test case in 
`PushDownProjectTest#testHasFreeLambdaArgument`.



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