Dmitry Lychagin has posted comments on this change. Change subject: [ASTERIXDB-1972][COMP][RT][TX] index-only plan ......................................................................
Patch Set 43: (4 comments) https://asterix-gerrit.ics.uci.edu/#/c/1866/43/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 67: * NEW: We need to cleanup this javadoc, so there's no longer NEW/OLD there, but a complete description of what happens in both cases (regular and index-only) Line 383: if (chosenIndexes.size() > 1) { This code is a bit confusing now. There used to be a special handling inside intersectAllSecondaryIndexes() for cases when there was only one chosen (secondary) index or when there was a primary index among chosen ones. Current code takes handling of a single chosen index out of intersectAllSecondaryIndexes() but keeps primary index handling there. I'd suggest either move everything (all new code) into intersectAllSecondaryIndexes() and just call it from here as it was done before, or take primary index handling out of intersectAllSecondaryIndexes() and do it here, so intersectAllSecondaryIndexes() is only about case when there're are several secondary indexes and no primary. Line 387: // Since only a single (secondary) index is chosen, checks whether this plan is an index-only plan. This comment is confusing. It says "single (secondary) index is chosen" (so no primary), but the comment later says "If the chosen index is the primary index" (so primary is allowed) Line 393: // If the chosen index is the primary index, adds variable:fieldname to subTree.fieldNames. This block is not going to be executed if the primary index was chosen along with secondary indexes, because that case would be handled inside intersectAllSecondaryIndexes() and that method does not have this block. So subTree.fieldNames won't be updated in this case. Is this a problem? -- 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: 43 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
