asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ildar Absalyamov (Code Review)" <>
Subject Change in asterixdb[master]: ASTERIXDB-1109: Fixed deletion of records from open secondar...
Date Sat, 17 Oct 2015 19:52:06 GMT
Ildar Absalyamov has posted comments on this change.

Change subject: ASTERIXDB-1109: Fixed deletion of records from open secondary index

Patch Set 6:

File asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/

Line 44: public class IntroduceMaterializationForInsertDeleteWithSelfScanRule implements IAlgebraicRewriteRule
> Actually, I'm not sure whether we need to have this materization rule for t
Initially I was thinking along those line as well and did not put materialization in delete
pipeline, but that resulted in quirky behavior deadlocking the index. I agree there might
be a better way of doing that, rather then materialization. Once the patch is merged I will
file a JIRA issue on that.

Line 82:     private boolean checkIfInsertDeleteAndScanDatasetsSame(AbstractLogicalOperator
op, String insertDatasetName) {
> insertDeleteDataSetName?
File asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/

Line 83:                 DeleteOperator insertDeleteOperator = (DeleteOperator) descendantOp;
> deleteOperator?
File asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/

Line 84: public class SubstitutePKFromDataScanOrUnnestRule implements IAlgebraicRewriteRule
> Wonderful rule! Generally, can we add more comments in the code itself? The
Should be better now, let me know if more explanation is needed

Line 99:         AssignOperator assign = (AssignOperator) op;
> assignOp?

Line 103:         for (Mutable<ILogicalExpression> e : assign.getExpressions()) {
> e -> expr?

Line 137:         AssignOperator newAssign = new AssignOperator(assign.getVariables(), newAssignExprs);
> newAssignOp?

Line 144:     private boolean checkIfSubstitutable(Mutable<ILogicalOperator> op, Mutable<ILogicalOperator>
> Especially for this function, please put more comments inside of this funct
File asterix-app/src/test/resources/optimizerts/queries/nested-index/scan-delete-secondary-index-btree-composite.aql:

Line 25:     l_orderkey: int32, 
> Can we remove these spaces?
File asterix-app/src/test/resources/optimizerts/results/disjunction-to-join-delete-1.plan:

Line 13:                         -- STREAM_PROJECT  |PARTITIONED|
> Why these deletions happened?
That's the result of a SubstitutePKFromDataScanOrUnnestRule firing: assigns were optimized
out, because they contained unnecessary PK lookups
File asterix-app/src/test/resources/optimizerts/results/nested-open-index/inverted-index-join/leftouterjoin-probe-pidx-with-join-edit-distance-check-idx_01.plan:

Line 78:                                                     -- SPLIT  |PARTITIONED|
> Actually here, below the SPLIT, they are not the same as the original opera
If you will look closely there are two SPILTs in this plan: lines 83 & 54 and lines 61.
Everything underneath here was deleted because it's a copy of lines 54-66.
It might be a bit confusing, cause the plan printer does not distinguish between SPLIT operators.
But I think generally we don't want to pull variable names into the plan, these is an old
issue about that

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I41bde91401f67918365de7df19dd2f0de20c73d2
Gerrit-PatchSet: 6
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ildar Absalyamov <>
Gerrit-Reviewer: Ian Maxon <>
Gerrit-Reviewer: Ildar Absalyamov <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Taewoo Kim <>
Gerrit-Reviewer: Till Westmann <>
Gerrit-Reviewer: Young-Seok Kim <>
Gerrit-HasComments: Yes

View raw message