924060929 commented on code in PR #69340:
URL: https://github.com/apache/doris/pull/69340#discussion_r4355886482
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java:
##########
@@ -576,11 +587,55 @@ public Void visitArrayMap(ArrayMap arrayMap,
CollectorContext context) {
Expression argument = arrayMap.getArgument(0);
if ((argument instanceof Lambda)) {
- return collectArrayPathInLambda((Lambda) argument, context);
+ Lambda lambda = (Lambda) argument;
+ return collectArrayMapPathInLambda(lambda, context);
}
return visit(arrayMap, context);
}
+ private Void collectArrayMapPathInLambda(Lambda lambda, CollectorContext
context) {
+ if (isRootContainerMetadata(context)) {
+ // A metadata requirement on the ARRAY_MAP result itself (OFFSET
for cardinality(arr),
+ // NULL for arr IS NULL) describes the output ARRAY container, not
the lambda elements.
+ // The output container is shaped by the input arrays: array_map
copies the first
+ // input's offsets and its null map is the union of all inputs'
null maps. Pushing that
+ // context into the lambda body would attach the metadata to the
element layer instead,
+ // producing e.g. `arr.*.OFFSET`, which the BE rejects for a
struct element (OFFSET
+ // metadata is only supported at array/map/string level). Route
the container metadata
+ // to the input arrays, and collect the element paths the lambda
actually reads with a
+ // fresh data context so that the elements stay available to the
lambda body.
+ // A map_keys / map_values input is normalized to its map column:
the produced array's
+ // container is the map itself, so the metadata must not be
attached to the KEYS /
+ // VALUES sub-column (which has no offsets of its own, e.g. for a
struct value).
+ for (ArrayItemReference lambdaArgument :
lambda.getLambdaArguments()) {
+ continueCollectAccessPath(
+
normalizeMapArrayProducer(lambdaArgument.getArrayExpression()),
+ copyContext(context));
+ }
+ return collectArrayPathInLambda(lambda,
+ new CollectorContext(context.statementContext,
context.bottomFilter));
+ }
+ // Only a root request is routed to the input containers. A prefixed
request such as the
+ // `[*, OFFSET]` of cardinality(element_at(array_map(...), 1))
addresses an element of the
+ // ARRAY_MAP result, whose shape is produced by the lambda and is
unrelated to the input
+ // array's containers. Keep the lambda traversal for it: the
unrepresentable metadata is
+ // dropped and the element data the lambda reads is preserved.
+ return collectArrayPathInLambda(lambda, context);
Review Comment:
Non-blocking follow-up: the existing comparator-form `visitArraySort` still
passes the result's META context to `collectArrayPathInLambda`, so comparator
evaluation can lose its DATA dependencies.
Reproduced on locally built FE+BE at `06d50b36d3e`, with nullable `a
ARRAY<INT>` containing `[2,2]`:
```sql
SELECT array_sort((x,y) -> IF(x IS NULL,0,y), a) IS NULL
FROM t;
```
- Pruning disabled: fails with `array_sort comparator returns 2, but it must
return -1, 0 or 1`.
- Pruning enabled: returns `false`; EXPLAIN reads only `[a.*.NULL, a.NULL]`.
- Both comparator arguments are referenced, so the unused-argument full-DATA
fallback does not mask this case. NULL-only reading replaces the actual
non-NULL integers with 0, hiding the required comparator error.
- Also projecting `a` restores the error with either setting. Literal
`[2,2]` errors with either setting, and the `[0,0]`/NULL/empty-array controls
agree.
Please collect the comparator with a fresh DATA context while keeping
`resultContext` for the output requirements. This is pre-existing in the merge
base and does not block this PR. The table-backed on/off outcomes were
reproduced three times each; this demonstrates changed error semantics, not
incorrect ordering for ordinary valid comparators or a crash.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java:
##########
@@ -576,11 +587,55 @@ public Void visitArrayMap(ArrayMap arrayMap,
CollectorContext context) {
Expression argument = arrayMap.getArgument(0);
if ((argument instanceof Lambda)) {
- return collectArrayPathInLambda((Lambda) argument, context);
+ Lambda lambda = (Lambda) argument;
+ return collectArrayMapPathInLambda(lambda, context);
}
return visit(arrayMap, context);
}
+ private Void collectArrayMapPathInLambda(Lambda lambda, CollectorContext
context) {
+ if (isRootContainerMetadata(context)) {
+ // A metadata requirement on the ARRAY_MAP result itself (OFFSET
for cardinality(arr),
+ // NULL for arr IS NULL) describes the output ARRAY container, not
the lambda elements.
+ // The output container is shaped by the input arrays: array_map
copies the first
+ // input's offsets and its null map is the union of all inputs'
null maps. Pushing that
+ // context into the lambda body would attach the metadata to the
element layer instead,
+ // producing e.g. `arr.*.OFFSET`, which the BE rejects for a
struct element (OFFSET
+ // metadata is only supported at array/map/string level). Route
the container metadata
+ // to the input arrays, and collect the element paths the lambda
actually reads with a
+ // fresh data context so that the elements stay available to the
lambda body.
+ // A map_keys / map_values input is normalized to its map column:
the produced array's
+ // container is the map itself, so the metadata must not be
attached to the KEYS /
+ // VALUES sub-column (which has no offsets of its own, e.g. for a
struct value).
+ for (ArrayItemReference lambdaArgument :
lambda.getLambdaArguments()) {
+ continueCollectAccessPath(
+
normalizeMapArrayProducer(lambdaArgument.getArrayExpression()),
+ copyContext(context));
+ }
+ return collectArrayPathInLambda(lambda,
Review Comment:
Non-blocking follow-up: the fresh DATA traversal here is also needed in the
existing `visitMapContainsKey/Value/Entry` NULL-only branches, which currently
return after visiting only the map.
I built and ran FE+BE at `06d50b36d3e` (macOS arm64, ASAN). With nullable `s
STRUCT<a:STRING,b:INT>`, nullable `m MAP<STRING,INT>`, and a row `s.a='123',
m={'k':123}`, set `enable_strict_cast=true` and run:
```sql
SELECT element_at(s,'a') IS NULL,
map_contains_value(m, CAST(element_at(s,'a') AS INT)) IS NULL
FROM t;
```
- `enable_prune_nested_column=false`: returns `false,false`.
- `true`: fails with `parse number fail, string: ''`.
- EXPLAIN with pruning has only `s.a.NULL` and `m.NULL`. The NULL-only
reader substitutes an empty string, but BE still executes the CAST.
- Projecting `s.a` as DATA instead of its IS NULL check restores success
with either setting.
- The equivalent key lookup using `MAP<INT,INT>` and entry lookup reproduce
the same failure.
Please collect all lookup arguments with independent DATA contexts even in
the NULL-only branches. This code already exists in the merge base; I suggest a
separate fix and am approving this ARRAY_MAP correction. The on/off outcomes
were reproduced three times each, including reversed execution order.
--
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]
