You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by "Young-Seok Kim (Code Review)" <do...@asterixdb.incubator.apache.org> on 2016/07/09 00:15:46 UTC
Change in asterixdb[master]: Index-only plan
Young-Seok Kim has posted comments on this change.
Change subject: Index-only plan
......................................................................
Patch Set 11:
(21 comments)
The following three issues are discovered.
1. R-tree index on rectangle type is not correctly dealt with for the corresponding index-only plan case
This case should be added as a test case.
2. limit-push-down for primary index scan doesn't seem to work.
3. limit-push-down for primary-index search doesn't seem to work.
Also, please address other comments.
https://asterix-gerrit.ics.uci.edu/#/c/744/11/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractReplicateOperator.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractReplicateOperator.java:
Line 36: private boolean[] outputMaterializationFlags = new boolean[outputArity];
I wonder why this flag is required.
https://asterix-gerrit.ics.uci.edu/#/c/744/11/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveRedundantGroupByDecorVarsRule.java
File algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveRedundantGroupByDecorVarsRule.java:
Line 45: public class RemoveRedundantGroupByDecorVarsRule implements IAlgebraicRewriteRule {
Is this class introduced for just for optimization, not for the correctness issue?
https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/PrimaryIndexSearchOperationDatasetLevelCallback.java
File asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/PrimaryIndexSearchOperationDatasetLevelCallback.java:
Line 35: public class PrimaryIndexSearchOperationDatasetLevelCallback extends AbstractOperationCallback implements ISearchOperationCallback {
This class should be removed.
https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/SecondaryIndexInstantSearchOperationTryLockOnlyOnPKCallback.java
File asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/SecondaryIndexInstantSearchOperationTryLockOnlyOnPKCallback.java:
Line 33: public class SecondaryIndexInstantSearchOperationTryLockOnlyOnPKCallback extends AbstractOperationCallback
Can we give a simpler class name, e.g., SecondaryIndexInstantSearchOperationCallback?
https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AbstractIntroduceAccessMethodRule.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AbstractIntroduceAccessMethodRule.java:
Line 644: .getIxJoinOuterAdditionalDataSourceVariables(i);
What's the difference between subTreeDataSourceVars and subTreePKs? why is it OK for subTreeDataSourceVars to replace some of subTreePks below?
Line 792: expr = (AbstractLogicalExpression) childFuncExpr.getArguments().get(0).getValue();
better be dealing with an else case explicitly, i.e., neither Assign nor unnest case.
https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodAnalysisContext.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodAnalysisContext.java:
Line 65: // whether a verification (especially for R-Tree case) is required after the secondary index search?
what's the difference between 3. requireVerificationAfterSIdxSearch and 4. doesSIdxSearchGenerateNoFalsePositiveResults? It seems the same? It would be good to explain the difference with example cases.
Line 67: // Does the given index can cover all search predicates?
It seems that the line 67 and 69 should be switched??
https://asterix-gerrit.ics.uci.edu/#/c/744/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodJobGenParams.java
File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodJobGenParams.java:
Line 44: protected boolean isPrimaryIndex;
Please say that "isPrimaryIndex is a derived parameter from the index name, so this parameter is not included in the 8 (NUM_PARAMS) parameters written and read."
https://asterix-gerrit.ics.uci.edu/#/c/744/11/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 260: // If it is not granted, then we need to do a secondary index lookup, sort PKs,
Do we sort in this case? ( I thought we don't do sort for this trylockfailure case?)
Line 1301: ((AbstractFunctionCallExpression) createOriginalSpatialObjectExpr).getArguments().addAll(expressions);
Here, the createOriginalSpatialObjectExpr should be added to restoredSecondaryKeyFieldExprs and similarly assignRestoredSecondaryKeyFieldOp and restoredSecondaryKeyFieldVars should be set correctly. Index-only plan test cases including R-tree index on rectangle field should be added.
https://asterix-gerrit.ics.uci.edu/#/c/744/11/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractLogicalOperator.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractLogicalOperator.java:
Line 61: // The code used for canDecreaseCardinality() - whether this operator can decrease the input cardinality
Please explain the meaning of each enum value, especially for CANBEBOTH and NOTAPPLICABLE
Line 70: public enum canPreserveOrderCode {
Please explain the meaning of each enum value, especially for CANBEBOTH and NOTAPPLICABLE
https://asterix-gerrit.ics.uci.edu/#/c/744/11/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/LeftOuterJoinOperator.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/LeftOuterJoinOperator.java:
Line 83: return canPreserveOrderCode.CANBEBOTH;
Again, what's the meaning of CANBEBOTH, here?
https://asterix-gerrit.ics.uci.edu/#/c/744/11/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/TokenizeOperator.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/TokenizeOperator.java:
Line 188: return canDecreaseCardinalityCode.FALSE;
Empty string may not generate any token? If yes, should this be true?
https://asterix-gerrit.ics.uci.edu/#/c/744/11/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveRedundantGroupByDecorVarsRule.java
File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveRedundantGroupByDecorVarsRule.java:
Line 45: public class RemoveRedundantGroupByDecorVarsRule implements IAlgebraicRewriteRule {
Why this class is duplicated in hyracks and algebricks ?
https://asterix-gerrit.ics.uci.edu/#/c/744/11/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveRedundantVariablesRule.java
File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/RemoveRedundantVariablesRule.java:
Line 188: if (equivalentVars != null && !equivalentVars.isEmpty()) {
equivalentVars are not null for sure since it's created in line 186.
https://asterix-gerrit.ics.uci.edu/#/c/744/11/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesSearchCursor.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesSearchCursor.java:
Line 276: proceedFailCount += 1;
remove this value or enable only if the code runs in debugging mode
Line 282: proceedSuccessCount += 1;
remove this value or enable only if the code runs in debugging mode
Line 327: if (useProceedResult) {
remove this value or enable only if the code runs in debugging mode
https://asterix-gerrit.ics.uci.edu/#/c/744/11/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/BinaryHashSet.java
File hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/BinaryHashSet.java:
Line 40: public class BinaryHashSet {
Why is this class not shown in the checked out codebase??
--
To view, visit https://asterix-gerrit.ics.uci.edu/744
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa02c13d4fddd880e1ee9e85eef6577301fb4560
Gerrit-PatchSet: 11
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim <wa...@yahoo.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Taewoo Kim <wa...@yahoo.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Young-Seok Kim <ki...@gmail.com>
Gerrit-HasComments: Yes