>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: WIP: improving Updatestatment visitors ...................................................................... Patch Set 6: (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/c0ef5161_dcafb011?usp=email : PS6, Line 733: return new Pair<>(null, null); Add a comment saying why it's not implemented. 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/7fd94662_6115db3a?usp=email : PS6, Line 53: private Expression priorExpr; Rename to: valueExprs pathExprs https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/41b44d49_b02f77cb?usp=email : PS6, Line 57: private Expression pathExpr; What does pathExpr represent as compared to valueExprs & pathExprs? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/185fc23b_8bbee6e4?usp=email : PS6, Line 178: public boolean hasPathAndValueLists() { hasPathValueExprs https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/2511c506_428d4008?usp=email : PS6, Line 219: if (getPathList().isEmpty()) { Don't we need to check: pathList == null || pathList.isEmpty() 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/5c46c204_aa54a966?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 something? currentContextVariable = newGeneratedVar; https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/0e5feef7_6e1349d8?usp=email : PS6, Line 182: if (currentContextVariable != null) { When can currentContextVariable be null? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/a3692753_de1bad67?usp=email : PS6, Line 435: return expr.accept(new AbstractSqlppSimpleExpressionVisitor() { I could not follow what this code is trying to do: return expr.accept(new AbstractSqlppSimpleExpressionVisitor() { ... ... 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/31cca7ae_33c79424?usp=email : PS6, Line 244: return false; Add a comment saying why it's not implemented. 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/3a4d8a4b_be2c3f33?usp=email : PS6, Line 313: || (changeExpr.getPathExpr() != null && changeExpr.getPathExpr().accept(this, arg)) Aren't we missing the pathExprs and valueExprs? 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/813bad89_21454e24?usp=email : PS6, Line 179: return visit(changeExpr.getPriorExpr(), arg) || visit(changeExpr.getPathExpr(), arg) Aren't we missing pathExprs and valueExprs? 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/1231c3f4_fa0eceb4?usp=email : PS6, Line 538: throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, changeExpr.getSourceLocation()); Add a comment saying why it's not implemented. 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/2a9a12b9_d93bc5bc?usp=email : PS6, Line 495: throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, changeExpr.getSourceLocation()); Add a comment saying why it's not implemented. 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/a313b6ba_d7828fcc?usp=email : PS6, Line 415: throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, changeExpr.getSourceLocation()); Add a comment saying why it's not implemented. 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/7a43ec47_ab71b0e3?usp=email : PS6, Line 184: throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, changeExpr.getSourceLocation()); Add a comment saying why it's not implemented. File asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20644/comment/24ebe65f_73b5a656?usp=email : PS6, Line 3073: ChangeExpression Change() throws ParseException: Rename Change() to ChangeExpression() 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/61a36573_7511e1e7?usp=email : PS6, Line 185: } Where is this happening? if you mark it as true, then we won't add the left value here: if (!foundMatch) { addFieldToSubRecord(combinedType, leftName, leftValue, null, nestedLevel); } -- 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: 6 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: Mon, 15 Dec 2025 20:04:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
