>From Ali Alsuliman <ali.al.solai...@gmail.com>: Attention is currently required from: Vijay Sarathy. Ali Alsuliman 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 6: (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/d48e6f66_db40a0db 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 fields and then having to keep track of when an index is removed to remove from this map. You could do it in fetchSecondaryIndexPreferences. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/d0186444_8a8ce2b6 PS6, Line 413: Remove added empty line (always remove not needed changes like these to make the review easier). https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/5b4d7f53_3a45cb3b PS6, Line 582: || (annotationRemoved && (this instanceof IntroduceJoinAccessMethodRule)) I am not sure I understand what this means. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/d7e64e36_ef7b696a PS6, Line 641: private Class<? extends AbstractExpressionAnnotationWithIndexNames> inapplicableHintWarning( : IOptimizableFuncExpr optFuncExpr, IOptimizationContext context) { Change IAccessMethod.getSecondaryIndexPreferences() to return the annotation directly. Using something like "instanceof IntroduceSelectAccessMethodRule" is not accurate. Also, use the "HINT_STRING" from the subclasses of "AbstractExpressionAnnotationWithIndexNames" for your "param1". https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/29d0411b_b42b938b 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: if (warningCollector.shouldWarn()) { //warn } Also change "ignored" to "inapplicable" https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18177/comment/ee14bfe4_496ad7b5 PS6, Line 683: && (preferredIndexNames == null || !preferredIndexNames.contains(index.getIndexName())) "preferredIndexNames" is becoming more like "applicableIndexNames", correct? -- 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: 6 Gerrit-Owner: Vijay Sarathy <vijay.sara...@couchbase.com> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Vijay Sarathy <vijay.sara...@couchbase.com> Gerrit-CC: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Attention: Vijay Sarathy <vijay.sara...@couchbase.com> Gerrit-Comment-Date: Fri, 29 Mar 2024 15:05:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
