>From Vijay Sarathy <[email protected]>: Attention is currently required from: Ali Alsuliman. Vijay Sarathy has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177 )
Change subject: [ASTERIXDB-3358][COMP] Indexnl hint with index names not working correctly ...................................................................... Patch Set 8: Code-Review+1 (6 comments) File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AbstractIntroduceAccessMethodRule.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/93846da4_b6bd2940 PS6, Line 378: // Used to keep track of applicable indexes for each expression. Since index hints : // are specific to an expression, it is useful to have this mapping to validate : // index hints against the applicable indexes for each expression. : Map<IOptimizableFuncExpr, Set<Index>> exprAndApplicableIndexes = new HashMap<>(); > It feels like this should be done after instead of doing it inside the while > loop with every key fie […] Added a method createExprAndApplicableIndexesMap(), that is called after the while loop to do this. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/c9252e9b_73c9a718 PS6, Line 413: > Remove added empty line (always remove not needed changes like these to make > the review easier). Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/7c63323d_06812ea2 PS6, Line 582: || (annotationRemoved && (this instanceof IntroduceJoinAccessMethodRule)) > I am not sure I understand what this means. In the case of joins, when we have no hint (the inapplicable hint has been removed earlier), we want to remove all applicable indexes, so that no indexnl will be tried with RBO, and all possible join methods will be tried with CBO. In the case of scans, when the inapplicable hint has been removed, we do not want to remove any applicable indexes, as RBO will do an intersection of all applicable indexes. CBO will not see the hint as it has been removed, and will make a cost based selection of all the applicable indexes. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/07860577_90854cb7 PS6, Line 641: private Class<? extends AbstractExpressionAnnotationWithIndexNames> inapplicableHintWarning( : IOptimizableFuncExpr optFuncExpr, IOptimizationContext context) { > Change IAccessMethod.getSecondaryIndexPreferences() to return the annotation > directly. […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/45d7e398_8087092f PS6, Line 654: warningCollector.warn(Warning.of(optFuncExpr.getFuncExpr().getSourceLocation(), : org.apache.hyracks.api.exceptions.ErrorCode.INAPPLICABLE_HINT, param1, "ignored")); > Always check first if you should warn: […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/f0b9e97b_fa604072 PS6, Line 683: && (preferredIndexNames == null || !preferredIndexNames.contains(index.getIndexName())) > "preferredIndexNames" is becoming more like "applicableIndexNames", correct? preferredIndexNames comes from the user specified hint, and applicableIndexNames comes from the applicable indexes based on the predicate. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I603ae267efd137d4e9f3491be2a6bdcb1179eeac Gerrit-Change-Number: 18177 Gerrit-PatchSet: 8 Gerrit-Owner: Vijay Sarathy <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Vijay Sarathy <[email protected]> Gerrit-CC: Ali Alsuliman <[email protected]> Gerrit-Attention: Ali Alsuliman <[email protected]> Gerrit-Comment-Date: Tue, 23 Apr 2024 21:29:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Ali Alsuliman <[email protected]> Gerrit-MessageType: comment
