Steven Jacobs has posted comments on this change. Change subject: Enhanced Insert AQL ......................................................................
Patch Set 10: (6 comments) https://asterix-gerrit.ics.uci.edu/#/c/1150/10/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/extension/IExtensionStatement.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/extension/IExtensionStatement.java: Line 42: * Called when the {@code IQueryTranslator} encounters an extension statement. > Fix the javadoc? Done Line 52: void handle(IStatementExecutor statementExecutor, AqlMetadataProvider metadataProvider, > Should we change this to only pass IStatementExecutor and create getters fo The problem with doing it that way is that they aren't members of IStatementExecutor but rather variables created for compileAndExecute. It looks like from the current usages of IStatementExecutor that we could actually change this though. Should I make them member variables for IStatementExecutor? https://asterix-gerrit.ics.uci.edu/#/c/1150/10/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/CommitOperator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/CommitOperator.java: Line 63: > setVar is not used by core. add a comment that it is used by extensions? Th Done https://asterix-gerrit.ics.uci.edu/#/c/1150/10/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/UpsertCommitRuntime.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/operators/physical/UpsertCommitRuntime.java: Line 35: resourcePartition, true); > mmmmmm, wouldn't this cause a problem if someone attempts Upsert with retur My changes will make upsert be able to return results as well now, so this won't be here in the next patch https://asterix-gerrit.ics.uci.edu/#/c/1150/10/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java: Line 96: if (op0.getOperatorTag() != LogicalOperatorTag.EXTENSION_OPERATOR > I think that all rewrite rules should be extension-agnostic. If an extensio Please see previous responses to Till. There is sort of a name clash here in the sense that an "Extension Operator" has nothing to do with Asterix Extensions. https://asterix-gerrit.ics.uci.edu/#/c/1150/10/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 423: ILogicalOperator leafOperator = new ExtensionOperator(new CommitOperator(true)); > I just noticed this ExtensionOperator which seems to have been here for a l I hope I've addressed this in my longer comment, but if not I can go through it more in our next meeting. -- To view, visit https://asterix-gerrit.ics.uci.edu/1150 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I65789d2a861d15232dd29156a6987d0635ec6c94 Gerrit-PatchSet: 10 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Steven Jacobs <sjaco...@ucr.edu> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Carey <dtab...@gmail.com> 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
