Vladsz83 commented on code in PR #10910:
URL: https://github.com/apache/ignite/pull/10910#discussion_r1358308153


##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/hint/HintsConfig.java:
##########
@@ -77,9 +81,38 @@ private HintsConfig() {
     public static HintStrategyTable buildHintTable() {
         HintStrategyTable.Builder b = 
HintStrategyTable.builder().errorHandler(Litmus.IGNORE);
 
+        RelOptRule[] disabledRulesTpl = new RelOptRule[0];
+
         Arrays.stream(HintDefinition.values()).forEach(hintDef ->
-            b.hintStrategy(hintDef.name(), 
HintStrategy.builder(hintDef.predicate()).build()));
+            b.hintStrategy(hintDef.name(), 
HintStrategy.builder(hintPredicate(hintDef))
+                
.excludedRules(hintDef.disabledRules().toArray(disabledRulesTpl)).build()));
 
         return b.build();
     }
+
+    /**
+     * Adds hint options checker to {@link HintPredicate} if {@code hintDef} 
has rules to exclude.
+     *
+     * @return Hint predicate.
+     */
+    private static HintPredicate hintPredicate(HintDefinition hintDef) {
+        if (F.isEmpty(hintDef.disabledRules()))

Review Comment:
   Because hint might not be used in any rule. Some hints may only bring 
dynamically new rules to the planner or mark some rules as skippable. Or use 
both approaches. Sice we do not use Calcite's `Litmus` and `HintErrorLogger` 
(they has own logger or requre dynamic logger initialization), we do not do 
general validation of hint params in this case. Usually, we do that in 
`HintUtils#filterHints()` . While the hint predicate is mandatory to call by 
Calcite to assing a hint to a rel node. Without wrapping of the hint predicate, 
`JoinOrderHintsPlannerTest#testWrongParams()` won't work.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to