github-actions[bot] commented on code in PR #67047:
URL: https://github.com/apache/doris/pull/67047#discussion_r3840407970
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java:
##########
@@ -712,7 +714,7 @@ private Void collectArrayPathInLambda(Lambda lambda,
CollectorContext context) {
// the complex column to null-only / offset-only instead of reading
full data.
//
// Detect usage by scanning the lambda body for ArrayItemSlots
matching the
- // argument name, which is more reliable than getInputSlots() that
deliberately
+ // argument ExprId, which is more reliable than getInputSlots() that
deliberately
Review Comment:
[P1] Keep full item access for lambda arguments that were not actually
collected
A syntactic `ArrayItemSlot` match does not prove that `visitArrayItemSlot`
ran. For example:
```text
Project[array_map((a, i) -> element_at(a, i), nested_arrays, indexes)]
Filter[indexes IS NOT NULL]
Scan[nested_arrays ARRAY<ARRAY<INT>>, indexes ARRAY<BIGINT>]
```
`visitElementAt` dispatches `a`, but calls the generic `visit(i, context)`
for the index. Since an already-`BIGINT` `i` remains a leaf, that call never
reaches `visitArrayItemSlot`. This loop still sees `i` syntactically and
suppresses its full-access fallback, leaving only `indexes.NULL` from the
filter (similarly `indexes.OFFSET` with `cardinality`). BE then uses a
metadata-only array read and empties/defaults the index payload even though
`element_at` needs it, causing an array-size error or wrong indices. Please
track successful slot resolution or dispatch non-container arguments through
`accept` with a fresh context, and add a multi-argument regression for this
case.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java:
##########
@@ -685,10 +687,10 @@ public Void visitNot(Not not, CollectorContext context) {
private Void collectArrayPathInLambda(Lambda lambda, CollectorContext
context) {
List<Expression> arguments = lambda.getArguments();
- Map<String, Expression> nameToArray = Maps.newLinkedHashMap();
+ Map<ExprId, Expression> exprIdToArray = Maps.newLinkedHashMap();
for (Expression argument : arguments) {
if (argument instanceof ArrayItemReference) {
- nameToArray.put(((ArrayItemReference) argument).getName(),
argument.child(0));
+ exprIdToArray.put(((ArrayItemReference) argument).getExprId(),
argument.child(0));
Review Comment:
[P1] Preserve the payload returned by comparator-form array_sort
Consider a scan column `arr ARRAY<ARRAY<INT>>` and:
```sql
array_sort((x, y) -> IF(cardinality(x) < cardinality(y), -1,
IF(cardinality(x) = cardinality(y), 0, 1)), arr)
```
Both lambda arguments are syntactically referenced, so the fallback adds no
full path; the comparator records only `arr.*.OFFSET`. Because `visitArraySort`
returns after this helper, nothing records that `array_sort` also returns the
original inner-array payload. BE consequently puts the inner iterator in
`OFFSET_ONLY`, preserves lengths for the comparator, but fills the returned
integers with defaults. For input `[[2,3,1],[4,2,1,4],[1,2]]`, the sorted
result can become zero-filled arrays instead of preserving those values. Please
add a full/result access path for comparator-form `array_sort` and cover a
scanned nested-array column, not only an array literal.
--
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]
