Taewoo Kim has posted comments on this change. Change subject: [ASTERIXDB-1972][COMP][RT][TX] index-only plan ......................................................................
Patch Set 56: (14 comments) Thanks! https://asterix-gerrit.ics.uci.edu/#/c/1866/56/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: PS56, Line 380: int keyType > let's create an enum for requested keys. 0,1,2 is not very informative Done Line 397: if (outputPrimaryKeysOnlyFromSIdxSearch) { > it's a bit strange that we need do cannot tell this information from the op Sure. Done. Line 414: // Fetches conditional splitter - the last position > let's add a sanity check here that unnestMapOp has getGenerateCallBackProce Done Line 416: stop = sourceVars.size(); > should we just set stop = start + 1 here? sourceVars might have more variab You are correct! Done. Line 427: public static List<LogicalVariable> getPrimaryKeyVarsFromSecondaryUnnestMap(Dataset dataset, > this method is no longer used. let's remove it. the functionality is provid Not sure why this method is not checked as unused on Eclipse. Removed. Line 514: if (realTypeConvertedToIntegerType) { > 1) it seems like this floor/ceil conversion is needed for index on closed-f 1) Done. 2) Yes. We don't have that protection now. Maybe we just check that case and throw an exception? Line 578: break; > throw IllegalStateException here. NEQ is not expected here. Done Line 583: if (mathFunctionTypeForNumericTypeCasting == TypeCastingMathFunctionType.NONE) { > this should probably be "else if" from the previous "if" because mathFuncti Got it. Done. Line 602: // We are optimizing a join query. Determine which variable feeds the secondary index. > this used to be in the 'else' branch. OK. I thought except a join case, the flow cannot reach here even without "elase". :-) Done. Line 949: List<LogicalVariable> skVarsFromSIdxUnnestMap = AccessMethodUtils.getKeyVarsFromSecondaryUnnestMap(dataset, > skVarsFromSIdxUnnestMap are used only for the index-only plan. let's move i Done Line 955: boolean skFieldUsedAfterTopOp = indexOnlyPlanInfo.getSecond(); > skFieldUsedAfterTopOp and skFieldUsedAfterTopOp are not used for non index- Done Line 965: // From now on, we deal with the index-only plan. > We now have createRestOfNonIndexOnlySearchPlan() method for dealing with no Done Line 1833: if (isInvertedIndex(chosenIndex)) { > Can we move this check to the top of this method? We know whether the index Good! Done. https://asterix-gerrit.ics.uci.edu/#/c/1866/56/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/OptimizableOperatorSubTree.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/OptimizableOperatorSubTree.java: Line 183: // Secondary index search in an index-only plan case > The assumption here is that if we see secondary index access then it's from Because of a new condition that you suggested, Rather than using dsType, I have created another function. -- 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: 56 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
