Dmitry Lychagin has posted comments on this change.

Change subject: [ASTERIXDB-1972][COMP][RT][TX] index-only plan
......................................................................


Patch Set 50:

(11 comments)

I'm not done yet, but here are a few more comments.

https://asterix-gerrit.ics.uci.edu/#/c/1866/50/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 960:         if (subTree.getDataSourceType() != 
DataSourceType.COLLECTION_SCAN) {
can datasourceType be INDEXONLY_PLAN_SECONDARY_INDEX_LOOKUP at this point? what 
would getDataSourceVariables() return for it?


Line 1033:             if (subTree.getDataset().getDatasetType() != 
DatasetType.EXTERNAL) {
you're looking for an internal dataset, right? Can we replace if condition with 
subTree.getDataset().getDatasetType() ==  DatasetType.INTERNAL


https://asterix-gerrit.ics.uci.edu/#/c/1866/50/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 1801:         
indexOnlyPlanInfo.setThird(requireVerificationAfterSIdxSearch);
there's no need to adding 3rd and 4th here, they'll be set at the end of this 
method anyway.


https://asterix-gerrit.ics.uci.edu/#/c/1866/50/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 169:         boolean doesSIdxSearchCoverAllPredicates = false;
this variable is not required. just pass 'false' to the Quadruple constructor.


Line 180:             isIndexOnlyPlan = false;
isIndexOnlyPlan is still 'false' at this point, so as first item in 
indexOnlyPlanInfo. So these two lines seem to be unnecessary


https://asterix-gerrit.ics.uci.edu/#/c/1866/50/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceSelectAccessMethodRule.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroduceSelectAccessMethodRule.java:

Line 406:                 } else if (chosenIndexes.size() > 1) {
this is minor. but can we switch the order of the last two else blocks. so if 
(chosenPrimaryIndex != null) { 
}
else if (choseIndexes.size() == 1) {
   // do index-only
}
else {
  // intersect secondary indexes.
}


https://asterix-gerrit.ics.uci.edu/#/c/1866/50/asterixdb/asterix-app/src/test/resources/optimizerts/results/rtree-secondary-index-open.plan
File 
asterixdb/asterix-app/src/test/resources/optimizerts/results/rtree-secondary-index-open.plan:

Line 16:     -- UNION_ALL  |PARTITIONED|
can you set noindexonly for this query, so the plan does not change?


https://asterix-gerrit.ics.uci.edu/#/c/1866/50/asterixdb/asterix-app/src/test/resources/optimizerts/results/rtree-secondary-index.plan
File 
asterixdb/asterix-app/src/test/resources/optimizerts/results/rtree-secondary-index.plan:

Line 16:     -- UNION_ALL  |PARTITIONED|
can you set noindexonly for this query, so the plan does not change?


https://asterix-gerrit.ics.uci.edu/#/c/1866/52/asterixdb/asterix-app/src/test/resources/optimizerts/results/udfs/query-ASTERIXDB-1019.plan
File 
asterixdb/asterix-app/src/test/resources/optimizerts/results/udfs/query-ASTERIXDB-1019.plan:

Line 36:                                                 -- UNION_ALL  
|PARTITIONED|
Add set noindexonly false for this query, so the plan does not change.


https://asterix-gerrit.ics.uci.edu/#/c/1866/52/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/utils/Quadruple.java
File 
hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/utils/Quadruple.java:

Line 69:         return first.hashCode() * 71 + second.hashCode() * 31 + 
third.hashCode() * 17 + fourth.hashCode();
use Objects.hash() as Pair/Triple do


Line 78:         return first.equals(quadRuple.first) && 
second.equals(quadRuple.second) && third.equals(quadRuple.third)
Use Objects.equals() as Pair/Triple do (it handles 'null' values)


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1866
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd5c9ab1cf2e4bedb7d8db582441919875e74d51
Gerrit-PatchSet: 50
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim <wangs...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com>
Gerrit-Reviewer: Ildar Absalyamov <ildar.absalya...@gmail.com>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Taewoo Kim <wangs...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to