Till Westmann has posted comments on this change. Change subject: Enhanced Insert AQL Added "return records" and "returning [fieldName]" "return records" returns to the user all records that were inserted "returning [fieldName]" returns only the valuse inserted for [fieldName] Allow commits to be non-sink operators (con ......................................................................
Patch Set 9: (15 comments) There seem to be a few unrelated modifications in this change and there are a few points that I'm not sure about: - Why is ExtensionStatement needed for this change? (I'm especially suspicious about the decisions taken in the rewriting rules base on the fact that a statement in an ExtensionStatement) - Why do we need to modify IExtensionStatement? - What is the relationship between ExtensionStatement and IExtensionStatement? https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-active/src/main/java/org/apache/asterix/active/IActiveEntityEventsListener.java File asterixdb/asterix-active/src/main/java/org/apache/asterix/active/IActiveEntityEventsListener.java: Line 34: public boolean isEntityUsingDataset(String dataverseName, String datasetName); Seems unrelated .. https://asterix-gerrit.ics.uci.edu/#/c/1150/9/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 54: int resultSetIdCounter) throws HyracksDataException, AlgebricksException; Seems unrelated as well ... https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceDynamicTypeCastRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceDynamicTypeCastRule.java: Line 101: if (op1.getOperatorTag() == LogicalOperatorTag.EXTENSION_OPERATOR) { This seems unrelated as well ... https://asterix-gerrit.ics.uci.edu/#/c/1150/9/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 Seems unrelated. https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceStaticTypeCastForInsertRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceStaticTypeCastForInsertRule.java: Line 96: if (op1.getOperatorTag() == LogicalOperatorTag.EXTENSION_OPERATOR) { Seems unrelated ... https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetupCommitExtensionOpRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/SetupCommitExtensionOpRule.java: Line 66: ExtensionOperator eOp = (ExtensionOperator) op; Seems unrelated. https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/UnnestToDataScanRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/UnnestToDataScanRule.java: Line 26: import org.apache.asterix.external.feed.watch.FeedActivityDetails; Seems unrelated. https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java: Line 305: throws AlgebricksException { Revert this file? https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java: Line 86: AlgebricksBuiltinFunctions.GE, AlgebricksBuiltinFunctions.LT, AlgebricksBuiltinFunctions.GT)); There seems to be no semantic change in this file, can we revert it? https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/OptimizableOperatorSubTree.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/OptimizableOperatorSubTree.java: Line 108: assignsAndUnnestsRefs.add(subTreeOpRef); This seems to be a step back in terms of extensibility, could we revert the file? https://asterix-gerrit.ics.uci.edu/#/c/1150/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 423: ILogicalOperator leafOperator = new ExtensionOperator(new CommitOperator(true)); Why do we need an ExtensionOperator here? https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java: Line 399: ((IExtensionStatement) stmt).handle(this, metadataProvider, hcc, hdc, resultDelivery, stats, Is this related? https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/runtime/ExecutionTestUtil.java File asterixdb/asterix-app/src/test/java/org/apache/asterix/test/runtime/ExecutionTestUtil.java: Line 60: AsterixHyracksIntegrationUtil alternateIntegrationUtil, boolean startHdfs) throws Exception { What is alternateIntegrationUtil? https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-doc/src/site/markdown/aql/manual.md File asterixdb/asterix-doc/src/site/markdown/aql/manual.md: Line 839: The optional "returning Identifier" returns only the value inserted for the field Identified by Identifier. These two options are helpful when using auto-generated keys, as they provide the user with a way to refer to the record in later queries. Looking at this(and the implementation) more closely, I'm not convinced that supporting these 2 special cases (whole record, 1 field) is the right solution. I think that we should aim to support any expression on each individual item that gets inserted. That has the advantage that a) we can do transformations on the item and that b) this also works if we decide that we can insert other items than records. To do this, we'd need a way to refer to the inserted item. One way to do this would be to introduce a special variable (e.g. $$) that can be used in the return-clause and another one would be to introduce a place to provide a name for the variable e.g. insert $b into dataset b ( for $a in dataset a where $a.x > 5 return $a ) returning func($b) or insert $b into dataset b returning func($b) ( for $a in dataset a where $a.x > 5 return $a ) While both of these could functionally do what I intend, none of them looks very elegant ... maybe there's a better alternative ... https://asterix-gerrit.ics.uci.edu/#/c/1150/9/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AUUID.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AUUID.java: Line 98: StringBuilder buf = new StringBuilder(UUID_CHARS + 9); Why 9 extra chars here? -- 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: 9 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Steven Jacobs <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
