github-actions[bot] commented on code in PR #69938:
URL: https://github.com/apache/doris/pull/69938#discussion_r4445027470
##########
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:
[P2] Avoid copying every enclosing lambda binding
Consider this reduced bound plan, with each `F_i` expanded inline and `A_i`
alternating between the two `ARRAY<BIGINT>` inputs:
```text
Project(F_1 AS v)
CrossJoin(Scan L(arr), Scan R(arr))
F_i = array_sum(array_map(x_i -> x_i + F_(i+1), A_i))
F_n = array_sum(array_map(x_n -> x_n, A_n))
```
Every `ArrayMap` is closed, but every suffix of length at least two uses
both join children. Both extraction callbacks therefore run
`hasFreeLambdaArgument` in full before their child-output containment check
rejects that candidate, then descend into the next suffix. At a suffix
containing `m` lambdas, this fresh builder's
[`addAll`](https://guava.dev/releases/33.2.1-jre/api/docs/com/google/common/collect/ImmutableSet.Builder.html)
re-adds `0 + ... + (m-1)` enclosing IDs. Across `n` candidates that is
`n(n+1)(n-1)/6` additions: 35,999,900 at `n=600`, whose bound expression has
depth 2402 and width 1200, below the configured defaults of 3000/10000. Each
body uses its own parameter, so the cached type-presence check cannot skip
these scans; `ArraySum` keeps the return type scalar, so nested array types do
not exclude the shape.
This adds cubic planner CPU/copying work to a linear-size expression; the
previous cached input-slot collection was quadratic across these suffixes.
Track bindings with scoped add/remove or equivalent free-variable summaries
instead of recopying every ancestor set. Checking ordinary child dependencies
before this scope scan also skips the ineligible cross-child candidates. These
counts are derived statically, not benchmark timings.
--
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]
