Yingyi Bu has posted comments on this change. Change subject: ASTERIXDB-1755: add UPSERT in SQL++. ......................................................................
Patch Set 9: (14 comments) https://asterix-gerrit.ics.uci.edu/#/c/1401/9//COMMIT_MSG Commit Message: Line 9: Detailed list of changes include: > s/include/included/? Done https://asterix-gerrit.ics.uci.edu/#/c/1401/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/CheckInsertUpsertRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/CheckInsertUpsertRule.java: Line 3: * * Licensed to the Apache Software Foundation (ASF) under one > Double indentation of comment. Done Line 37: public class CheckInsertUpsertRule implements IAlgebraicRewriteRule { > Should this be called CheckReturningRule? It's really about RETURNING and n Done https://asterix-gerrit.ics.uci.edu/#/c/1401/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/util/InsertUpsertCheckUtil.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/util/InsertUpsertCheckUtil.java: Line 3: * * Licensed to the Apache Software Foundation (ASF) under one > Double indentation of comment. Done Line 39: > Remove the empty line? Done https://asterix-gerrit.ics.uci.edu/#/c/1401/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/AqlExpressionToPlanTranslator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/AqlExpressionToPlanTranslator.java: Line 175: if (expr.getKind() == Kind.FLWOGR_EXPRESSION) { > reuse "isFLWOGR" ? Done https://asterix-gerrit.ics.uci.edu/#/c/1401/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java: Line 567: if (compiledUpsert.getReturnExpression() != null) { > Reuse "returnExpression" declared above? Done Line 629: rootOperator = new DelegateOperator(new CommitOperator(returnExpression == null ? true : false)); > s/returnExpression == null ? true : false/returnExpression == null/ ? Done Line 645: rootOperator.getInputs().add(new MutableObject<>(upsertOp)); > Could we pull these last 2 lines behind the else? Done https://asterix-gerrit.ics.uci.edu/#/c/1401/9/asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/rewrites/AqlQueryRewriter.java File asterixdb/asterix-lang-aql/src/main/java/org/apache/asterix/lang/aql/rewrites/AqlQueryRewriter.java: Line 61: private void setup(List<FunctionDecl> declaredFunctions, StatementWithReturn topExpr, > Rename parameter to "topStatement" (or "topStmt") as well? Done Line 71: public void rewrite(List<FunctionDecl> declaredFunctions, StatementWithReturn topExpr, > Rename parameter to "topStatement" (or "topStmt") as well? Done https://asterix-gerrit.ics.uci.edu/#/c/1401/9/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/base/StatementWithReturn.java File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/base/StatementWithReturn.java: Line 3: * * Licensed to the Apache Software Foundation (ASF) under one > Double indentation of comment. Done Line 30: public interface StatementWithReturn extends Statement { > Maybe rename this to "IReturningStatement"? Done https://asterix-gerrit.ics.uci.edu/#/c/1401/9/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppExpressionScopingVisitor.java File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppExpressionScopingVisitor.java: Line 365: Expression returningExpr = insertStatement.getReturnExpression(); > Move this down to the place where it is used? Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1401 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02b2be7ff2653573eccb48037895f5c8c4bc8c74 Gerrit-PatchSet: 9 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Steven Jacobs <sjaco...@ucr.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
