Taewoo Kim has posted comments on this change.

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


Patch Set 52:

(11 comments)

Thanks!!

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? 
DataSource can be INDEXONLY_PLAN_SECONDARY_INDEX_LOOKUP for the outer branch in 
case the outer branch is an index-only plan. In this case, it only returns the 
primary key. I have fixed this part.


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


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 th
In case this is not the index-only plan, the flow doesn't reach there. Please 
see the next three lines.


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 construct
Done


Line 180:             isIndexOnlyPlan = false;
> isIndexOnlyPlan is still 'false' at this point, so as first item in indexOn
Correct. Done.


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 
Done


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?
Done. I have added one more test case for the index-only version.


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?
Done. I have added the index-only version, too.


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.
Done


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
Done


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)
Done


-- 
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: 52
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Ildar Absalyamov <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Taewoo Kim <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-HasComments: Yes

Reply via email to