>From Shahrzad Shirazi <[email protected]>: Shahrzad Shirazi has posted comments on this change by Shahrzad Shirazi. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644?usp=email )
Change subject: WIP: improving Updatestatment visitors ...................................................................... Patch Set 9: (17 comments) File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/899f07cd_386f9ea9?usp=email : PS6, Line 733: return new Pair<>(null, null); > Add a comment saying why it's not implemented. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/ChangeExpression.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/6c9ec71d_79f42100?usp=email : PS6, Line 53: private Expression priorExpr; > Rename to: […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/ad419486_0d061c81?usp=email : PS6, Line 57: private Expression pathExpr; > What does pathExpr represent as compared to valueExprs & pathExprs? the pathExpr is the expr for keeping the target of inner insert/delete/update statements before we rewrite them to Valuelist and pathlist. like roles in the following example: UPDATE UserTypes (INSERT INTO UserTypes.roles AT 3([ {"roleId": "role3", "roleName": "reviewer"}])) SET UserTypes.age=33 WHERE userId = 1; https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/bbd383de_9721000a?usp=email : PS6, Line 178: public boolean hasPathAndValueLists() { > hasPathValueExprs Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/9ef48976_7ee9d8e7?usp=email : PS6, Line 219: if (getPathList().isEmpty()) { > Don't we need to check: pathList == null || pathList. […] 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/+/20644/comment/c940cf70_67ac2888?usp=email : PS6, Line 144: currentContextVariable = newGeneratedVar; > What's the purpose of assigning again since we already made this assignment > above or am I missing so […] correct not needed https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/1563b78a_e11fbb3a?usp=email : PS6, Line 182: if (currentContextVariable != null) { > When can currentContextVariable be null? Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/17eb2522_2d0d2d4f?usp=email : PS6, Line 435: return expr.accept(new AbstractSqlppSimpleExpressionVisitor() { > I could not follow what this code is trying to do: […] I added this for detecting and changing statements like SET u.totaltax= u.totalcost*1/100; to make sure that we are substituting all vars with generated vars File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/3ffc4f9e_75123c1a?usp=email : PS6, Line 244: return false; > Add a comment saying why it's not implemented. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/ccb32c40_817d7b0c?usp=email : PS6, Line 313: || (changeExpr.getPathExpr() != null && changeExpr.getPathExpr().accept(this, arg)) > Aren't we missing the pathExprs and valueExprs? Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/c82815b6_3d05771d?usp=email : PS6, Line 179: return visit(changeExpr.getPriorExpr(), arg) || visit(changeExpr.getPathExpr(), arg) > Aren't we missing pathExprs and valueExprs? Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/15ed7f11_4cc15fd0?usp=email : PS6, Line 538: throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, changeExpr.getSourceLocation()); > Add a comment saying why it's not implemented. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/e9f32bc0_c5012b95?usp=email : PS6, Line 495: throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, changeExpr.getSourceLocation()); > Add a comment saying why it's not implemented. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/94a94a68_df7bd05b?usp=email : PS6, Line 415: throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, changeExpr.getSourceLocation()); > Add a comment saying why it's not implemented. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppContainsExpressionVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/18265fa7_d6910291?usp=email : PS6, Line 184: throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, changeExpr.getSourceLocation()); > Add a comment saying why it's not implemented. Done File asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/e93f06c0_830b0d57?usp=email : PS6, Line 3073: ChangeExpression Change() throws ParseException: > Rename Change() to ChangeExpression() Done File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeEvaluator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/4eef8bdf_10dd04ad?usp=email : PS6, Line 185: } > Where is this happening? if you mark it as true, then we won't add the left > value here: […] was wrong changed it. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644?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: master Gerrit-Change-Id: I87c5622716b261ed6ae6001e0cd84568e6690eae Gerrit-Change-Number: 20644 Gerrit-PatchSet: 9 Gerrit-Owner: Shahrzad Shirazi <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Comment-Date: Tue, 16 Dec 2025 22:28:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ali Alsuliman <[email protected]>
