>From Shahrzad Shirazi <[email protected]>: Shahrzad Shirazi has posted comments on this change by Shahrzad Shirazi. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268?usp=email )
Change subject: [ASTERIXDB-3516][COMP] Adding UPDATE statement ...................................................................... Patch Set 44: (64 comments) File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetClosedRecordConstructorsRule.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/f60dd953_94e036dd?usp=email : PS42, Line 113: && !expr.hasAnnotation(IsTransformExpressionAnnotation.class)) { > IsTransformRecordAnnotation Done File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/1a1b59d7_1ad3f099?usp=email : PS41, Line 669: IResultMetadata resultMetadata, Statement.Kind kind) throws AlgebricksException { > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/ec81438f_f632f2b9?usp=email : PS41, Line 1691: f = new ScalarFunctionCallExpression( > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/0316a5ef_425a3a5b?usp=email : PS41, Line 2076: if (fce.getFunctionSignature().getName().equals(BuiltinFunctions.DATA_TRANSFORM.getName()) || > Done Done File asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/3a505fef_5be4e697?usp=email : PS41, Line 344: 1234 = Invalid update target, expected a list (ordered or unordered), but found: %s > Done Done File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/InsertStatement.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/861e2662_8843a7a3?usp=email : PS41, Line 104: if (getQuery() != null) { //if the query is not null it is an update statement > it always has a value Done File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/InsertStatement.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/4e4aae49_e67210c7?usp=email : PS42, Line 39: private Query query; > I would like to see if we can generate the Query as part of parsing the > UPDATE statement in SQLPP. […] Done File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/UpdateStatement.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/cb25b301_7a1e0ab1?usp=email : PS42, Line 36: Expression changeSeq, Expression condition, Expression returnExpression) { > Those two might be gone if we could generate the Query() in SQLPP. […] Done File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/FormatPrintVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/0f094f9b_08e2de13?usp=email : PS42, Line 687: update.getChangeSeq().accept(this, step + 2); > We might just need to call on getQuery() like InsertStatement and it should > take care of printing th […] 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/+/20268/comment/0e6ebbff_fc394814?usp=email : PS41, Line 48: public Expression getPriorExpr() { > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/e2e74c84_534b03c9?usp=email : PS41, Line 48: public Expression getPriorExpr() { > Done 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/+/20268/comment/87340ffd_54d04a46?usp=email : PS42, Line 34: private Expression setExpr; > Make any variable "final" for those that can be final. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SetExpression.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/9153a824_07d6827d?usp=email : PS41, Line 49: this.pathExpr = pathExpr != null ? pathExpr : new ArrayList<>(); > clear the list? Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SetExpression.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/6cea8424_ec058209?usp=email : PS42, Line 49: if (pathExpr != null) { > Just set directly since we know it's not going to be null. Same thing for > below. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/3a5f9f63_395073c8?usp=email : PS42, Line 183: setParameterCheckVisitor(); > checkUpdateSetExpressions(); Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SetParameterCheckVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/81e492b6_38cd8f66?usp=email : PS41, Line 88: if (pkPath.size() == 1 && pkPath.get(0).equals(fieldName)) { > Done Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SetParameterCheckVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/3dc18639_3441bbf3?usp=email : PS42, Line 54: private void addAccessedEntities(CallExpr expression) { > Override the UpdateStatement. Then, capture the Namespace and datasetName and > call super. […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/e39bfba9_bc13178a?usp=email : PS42, Line 97: for (List<String> pkPath : primaryKeys) { > See if we can use contains() Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppChangeSeqToSelectExprVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/4ece88e8_8eec89b3?usp=email : PS42, Line 111: return visit(selectExpression, changeSeqExpr); > arg? Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppChangeToSetExpressionVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/6090930f_81e9156c?usp=email : PS42, Line 130: if (changeExpr.getType() == ChangeExpression.UpdateType.UPDATE) { > Extract each one into a separate method. Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/ec97a1ef_bb4e832c?usp=email : PS42, Line 323: public Expression visit(ChangeExpression changeExpr, ILangExpression arg) throws CompilationException { > Organize the methods so that the overridden ones are next to each other. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/9f3a77d2_6f5c8d71?usp=email : PS41, Line 196: return null; > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/f9a8fe25_b7f4cac4?usp=email : PS41, Line 196: return null; > Done Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/VariableCheckAndRewriteVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/3c8ce044_27be6aa8?usp=email : PS42, Line 19: org.apache.asterix.lang.sqlpp.rewrites.v > What we have now […] 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/+/20268/comment/a99eabc0_5372d551?usp=email : PS41, Line 313: return false; > Done 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/+/20268/comment/30cb73e4_bf9b619e?usp=email : PS41, Line 180: return false; > Same comment as the one in the other visitors? 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/+/20268/comment/03273f9d_e919d511?usp=email : PS42, Line 540: return setexpr; > Throw illegal 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/+/20268/comment/992fe99d_88f6cd75?usp=email : PS42, Line 496: return null; > See if this can be called before those expressions are gone. […] They expressions are gone throwing errors and will implement later as you mentioned. File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/41bba21f_7a6874f5?usp=email : PS41, Line 392: > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/48eb7fd8_1404499b?usp=email : PS41, Line 454: > Done 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/+/20268/comment/fede3d58_95dae0c3?usp=email : PS42, Line 415: return new Pair<>(changeExpr, env); > Throw illegal or implement properly if they are going to be called. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/ffd4ed6b_4afb86a9?usp=email : PS41, Line 367: > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/2f10f0f3_971995e8?usp=email : PS41, Line 372: > Done Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppUpdateRewriteVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/646585fa_bb3dd0e7?usp=email : PS42, Line 53: public static final SqlppUpdateRewriteVisitor INSTANCE = new SqlppUpdateRewriteVisitor(); > If we are able to generate the Query as part of parsing in SQLPP.jj, then > this class can be removed. 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/+/20268/comment/804a0e9f_f4248e62?usp=email : PS41, Line 184: return true; > Done 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/+/20268/comment/30aa8f3b_e1789fa4?usp=email : PS42, Line 184: return false; > Throw illegal or implement for the expressions inside. Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppExpressionScopingVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/4395d87d_52b6a789?usp=email : PS41, Line 67: import org.apache.logging.log4j.util.Strings; > Done Done File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/08911688_bab45699?usp=email : PS41, Line 414: public Expression visit(ChangeExpression changeExpr, ILangExpression arg) throws CompilationException { > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/24f4e692_b74e9a19?usp=email : PS41, Line 415: changeExpr.setPriorExpr(visit(changeExpr.getPriorExpr(), changeExpr)); > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/aaab101d_acb7f44b?usp=email : PS41, Line 424: setexpr.setValueExprList(visit(setexpr.getValueExprList(), setexpr)); > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/4200e552_abe0539c?usp=email : PS41, Line 441: public Expression visit(org.apache.asterix.lang.common.statement.UpdateStatement updateStmt, ILangExpression arg) > Done Done File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/f9c66f7a_79fce160?usp=email : PS41, Line 269: FunctionConstants.newAsterix("data-transfer-object-constructor", FunctionIdentifier.VARARGS); > Done Done File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/BuiltinFunctions.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/3f5106d3_49c1e5ef?usp=email : PS42, Line 250: public static final FunctionIdentifier DATA_TRANSFORM = FunctionConstants.newAsterix("data-transform", 2); > object-transform Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/9cffc5ad_82573c4e?usp=email : PS42, Line 250: public static final FunctionIdentifier DATA_TRANSFORM = FunctionConstants.newAsterix("data-transform", 2); > RECORD_TRANSFORM Done File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/DataTransformTypeComputer.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/732b53d2_7a3851f5?usp=email : PS41, Line 46: ist0TransformRec = recType0.isTransform(); > Done Done File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/DataTransformTypeComputer.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/c0cd9f6f_9d9ec489?usp=email : PS42, Line 32: public class DataTransformTypeComputer implements IResultTypeComputer { > RecordTransformTypeComputer Done File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/RecursiveRemovalTypeComputer.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/e7a79f2d_1184822f?usp=email : PS42, Line 37: public class RecursiveRemovalTypeComputer implements IResultTypeComputer { > RecordRemoveRecursiveTypeComputer Done File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ARecordType.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/f10db2e5_6a04c320?usp=email : PS41, Line 71: private boolean isTransform; > Revert the change. Done File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/CheckListDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/1f8e2660_c603e098?usp=email : PS42, Line 72: if (PointableHelper.checkAndSetMissingOrNull(result, argPtr)) { > This should be removed. Done File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/CheckIntegerDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/cc889279_fc28b3b6?usp=email : PS41, Line 55: return new IScalarEvaluator() { > Done Done File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/CheckIntegerDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/0e262d07_32b30fd2?usp=email : PS42, Line 66: if (PointableHelper.checkAndSetMissingOrNull(result, argPtr)) { > Should be removed. Done File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/DataTransformDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/9a7ea52d_1af76955?usp=email : PS41, Line 68: argTypes[0] = (IAType) states[0]; > Done Done File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/DataTransformDescriptor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/53269cbc_a083dcfd?usp=email : PS42, Line 50: public class DataTransformDescriptor extends AbstractScalarFunctionDynamicDescriptor { > RecordTransformDescriptor Done File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/DataTransformEvaluator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/4ccd1352_16a6d45a?usp=email : PS41, Line 38: public class DataTransformEvaluator extends AbstractScalarEval { > Done Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/d4d90a33_875bb546?usp=email : PS41, Line 85: if (!leftRecType.isTransform()) { > Done Done File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/DataTransformEvaluator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/05c8739b_d91d32ee?usp=email : PS42, Line 81: ARecordType leftRecType = TypeComputeUtils.extractRecordType(argTypes[1]); > We don't need those. Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/31f70d4d_c20ba770?usp=email : PS42, Line 85: RecordMergeEvaluator mergeEval = new RecordMergeEvaluator( > Remove this and just call super. […] 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/+/20268/comment/a4a15790_4ca8ad40?usp=email : PS42, Line 121: RecordMergeEvaluator(IAType[] argTypes, IPointable[] args, SourceLocation sourceLocation, > This should be gone. Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/1046cf4d_0bdcbe13?usp=email : PS42, Line 156: vp0.set(argPtr0); > Move this back to its place and put inside if() Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/6dd82ead_dbdbbf08?usp=email : PS42, Line 184: RuntimeRecordTypeInfo leftRecordTypeInfo = new RuntimeRecordTypeInfo(); > We are creating this object per tuple. See how rbStack is used and let's do > the same thing. is it correct now ? File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecursiveRemovalEvaluator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/34c097be_230814b1?usp=email : PS42, Line 78: if (argRightTag != ATypeTag.OBJECT || argLeftTag != ATypeTag.OBJECT) { : result.set(argPtrRight); : return; : } > The difference between this evaluator and the DataTransformEvaluator is that > this is created for rem […] Done File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/functions/FunctionTypeInferers.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/6393f4a5_03889df2?usp=email : PS41, Line 305: public static final class DataTransformTypeInferer implements IFunctionTypeInferer { > Done Done File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/SubplanOperator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/66d61def_3981f938?usp=email : PS41, Line 40: private boolean isFailSafe = false; > Done Done File hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/SubplanRuntimeFactory.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268/comment/a073db64_f753dfea?usp=email : PS29, Line 284: smthWasWritten = false; > Done Done -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20268?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: Ib2d531b7b172b75b9756c7cc9b15dc636641f827 Gerrit-Change-Number: 20268 Gerrit-PatchSet: 44 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, 18 Nov 2025 22:03:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shahrzad Shirazi <[email protected]> Comment-In-Reply-To: Ali Alsuliman <[email protected]>
