Dmitry Lychagin has posted comments on this change. Change subject: [ASTERIXDB-2015][IDX] Introduce Primary Index Optimization Rule ......................................................................
Patch Set 6: (6 comments) https://asterix-gerrit.ics.uci.edu/#/c/2111/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroducePrimaryIndexForAggregationRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroducePrimaryIndexForAggregationRule.java: Line 60: * aggregate operator (local) It seems to me that an aggregate followed by assign is really not a prerequisite for applying this transformation. Any data-scan/index-search over the primary index can be replaced with secondary primary index search as long as only the primary key is used by the query (this rule checks for it in scanOperatorVariablesAreUsed()). Is that right? Was this discussed? Line 91: private final LinkedList<Mutable<ILogicalOperator>> parents; Let's make field type List<> and use ArrayList<> as implementation, so we avoid object creation on every add() call. Line 139: // TODO(ali): can it happen that an aggregate operator has more than one input? I think an aggregate operator can only have one input. Line 156: UnnestingFunctionCallExpression functionCallExpression = Use AbstractFunctionCallExpression here instead of UnnestingFunctionCallExpression and check whether the function is BuiltinFunctions.INDEX_SEARCH Line 195: boolean requiresBroadcast = scanOperator.getInputs().get(0).getValue().getExecutionMode() == If you're replacing index-search() then I think you can just get requiresBroadcast and retainInput from it (from originalBTreeParameters) For retainInput in case of a data-scan you probably need to do the analysis using AccessMethodUtils.retainInputs() as your comment indicates. I think you have all the information needed for it. Line 208: newBTreeParameters.setIsEqCondition(true); // TODO(ali): check this This is not an 'eq' condition. We're getting everything from the index. -- To view, visit https://asterix-gerrit.ics.uci.edu/2111 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3bbb2b5e1f25e61928d73b866e91c592ce0bf954 Gerrit-PatchSet: 6 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
