>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/+/20644?usp=email ) Change subject: [ASTERIXDB-3516][COMP] Improving the visitors for UPDATE statement ...................................................................... Patch Set 10: (9 comments) 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/8fe06cfc_64ce2282?usp=email : PS10, Line 59: private SetExpressionTree exprTree; IntelliJ is showing that "exprTree" could be final. Take a look at the warnings and address them. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/3f3f04b6_52593fb6?usp=email : PS10, Line 62: private Expression deleteUpateInsertTargetExpr; Fix typo: deleteUpdateInsertTargetExpr, or better just changeTargetExpr https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/9d490709_4da80f2f?usp=email : PS10, Line 139: public Expression getDeleteUpateInsertTargetExpr() { getChangeTargetExpr() https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/92b2e527_c1f78753?usp=email : PS10, Line 167: public void setDeleteUpateInsertTargetExpr(Expression deleteUpateInsertTargetExpr) { setChangeTargetExpr(Expression changeTargetExpr) https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/f949a5d3_49e4b9ec?usp=email : PS10, Line 207: public boolean hasPathExpr() { hasChangeTargetExpr() File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/parser/SetExpressionTree.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/8d2b238c_4a4b1698?usp=email : PS10, Line 74: public class SetExpressionTree { IntelliJ is showing a couple of warnings. Take a look and address them. They are easy ones like: 1. 'children.size() > 0' can be replaced with '!children.isEmpty() 2. Value of parameter 'createIfEmpty' is always 'true' 3. Inner class 'Node' may be 'static' Also, remove unused methods. File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/f596a3b2_c355ba89?usp=email : PS10, Line 179: rewriteUpdateModificationExpressions(); With all the renaming we did, check if a better name for this could be: rewriteUpdateChangeExpressions() 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/2d077368_68e5fa5c?usp=email : PS10, Line 321: || (changeExpr.getValueExprs() != null && visitExprList(changeExpr.getValueExprs(), arg)) We are missing the pathExprs. File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/4df991ac_7f827197?usp=email : PS10, Line 361: if (changeExpr.getPathExprs().size() != 0) { Try to be consistent. In 'SqlppAstPrintVisitor', you are using: if (changeExpr.hasPathValueExprs()) -- 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: 10 Gerrit-Owner: Shahrzad Shirazi <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Attention: Shahrzad Shirazi <[email protected]> Gerrit-Comment-Date: Wed, 17 Dec 2025 02:24:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
