>From Shahrzad Shirazi <[email protected]>: Attention is currently required from: Ali Alsuliman.
Shahrzad Shirazi has posted comments on this change by Shahrzad Shirazi. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21096?usp=email ) Change subject: [ASTERIXDB-3759][COMP] Fix inconsistent array index handling in UPDATE AT clause ...................................................................... Patch Set 20: Code-Review+1 (5 comments) File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppChangeExprToSelectExprVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21096/comment/225d4e3f_e5828d67?usp=email : PS17, Line 250: if (cond.getKind() == Expression.Kind.OP_EXPRESSION) { > Shouldn't we be looking for expressions that are using `changeExpr. […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21096/comment/a50c9875_a88453a1?usp=email : PS17, Line 332: if (condition.getKind() == Expression.Kind.OP_EXPRESSION) { > Feels like duplicate of the above one? We should refactor if yes. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppChangeExprToSelectExprVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21096/comment/fd01dae2_8658377e?usp=email : PS20, Line 82: SET u.addresses = CASE check-list(u.addresses) WHEN true THEN array-concat( : * (SELECT element $addresses FROM u.addresses AS The change here is needed because based on the docs when we have multiple records—some with lists and some without, we need to apply the updates only to the records with lists, keep the others unchanged, and return a warning for those that were not modified. File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/CheckInsertPositionDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21096/comment/e2c4d431_3ab8119c?usp=email : PS17, Line 67: if (PointableHelper.checkAndSetMissingOrNull(result, posPtr, lenPtr)) { > How can we get MISSING/NULL? If that's possible (e.g. […] No it can not be that when we get to this function.The position and the length of the list are handled by check-integer and check-list before we get to this . https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21096/comment/517ccc1a_5527d396?usp=email : PS17, Line 74: if ((position < 0 || position > arrayLength) && ctx.getWarningCollector().shouldWarn()) { > We should have regression tests to test the edge cases like when the index is > at the beginning or at […] Done -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21096?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: lumina Gerrit-Change-Id: I186b0cd683416cc843671a7812f7c8551a36ced4 Gerrit-Change-Number: 21096 Gerrit-PatchSet: 20 Gerrit-Owner: Shahrzad Shirazi <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Shahrzad Shirazi <[email protected]> Gerrit-Attention: Ali Alsuliman <[email protected]> Gerrit-Comment-Date: Thu, 07 May 2026 18:13:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Ali Alsuliman <[email protected]>
