>From Ali Alsuliman <[email protected]>: Attention is currently required from: Shahrzad Shirazi.
Ali Alsuliman 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 17: Code-Review+2 (4 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/a410763e_45000f93?usp=email : PS17, Line 250: if (cond.getKind() == Expression.Kind.OP_EXPRESSION) { Shouldn't we be looking for expressions that are using `changeExpr.getPosVar()`? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21096/comment/dc979827_5c7f219d?usp=email : PS17, Line 332: if (condition.getKind() == Expression.Kind.OP_EXPRESSION) { Feels like duplicate of the above one? We should refactor if yes. 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/670f3cab_be7df203?usp=email : PS17, Line 67: if (PointableHelper.checkAndSetMissingOrNull(result, posPtr, lenPtr)) { How can we get MISSING/NULL? If that's possible (e.g. someone is calling a function to compute the index, but the computation returns NULL), then what do we say our behavior should be (is it in the PRD)? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21096/comment/6733ec7d_333c2f26?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 the end. -- 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: 17 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: Shahrzad Shirazi <[email protected]> Gerrit-Comment-Date: Tue, 05 May 2026 17:32:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes
