[GitHub] [calcite] beyond1920 commented on a change in pull request #2664: [CALCITE-4967] Support SQL hints for temporal table join

[GitBox <gi...@apache.org>]

beyond1920 commented on a change in pull request #2664:
URL: https://github.com/apache/calcite/pull/2664#discussion_r785583739



##########
File path: core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java
##########
@@ -771,30 +786,49 @@ static HintStrategyTable createHintStrategies(HintStrategyTable.Builder builder)
                         + "allowed options: [ONE_PHASE, TWO_PHASE]",
                     hint.hintName)).build())
         .hintStrategy("use_hash_join",
-          HintPredicates.and(HintPredicates.JOIN, joinWithFixedTableName()))
+          HintPredicates.or(
+              HintPredicates.and(HintPredicates.CORRELATE, withFixedTableName()),
+              HintPredicates.and(HintPredicates.JOIN, withFixedTableName())))
         .hintStrategy("use_merge_join",
             HintStrategy.builder(
-                HintPredicates.and(HintPredicates.JOIN, joinWithFixedTableName()))
+                HintPredicates.and(HintPredicates.JOIN, withFixedTableName()))
                 .excludedRules(EnumerableRules.ENUMERABLE_JOIN_RULE).build())
         .build();
     }
 
-    /** Returns a {@link HintPredicate} for join with specified table references. */
-    private static HintPredicate joinWithFixedTableName() {
+    /** Returns a {@link HintPredicate} for join or correlate with specified table references. */
+    private static HintPredicate withFixedTableName() {
       return (hint, rel) -> {
-        if (!(rel instanceof LogicalJoin)) {
+        if (!(rel instanceof LogicalJoin || rel instanceof LogicalCorrelate)) {
           return false;
         }
-        LogicalJoin join = (LogicalJoin) rel;
         final List<String> tableNames = hint.listOptions;
-        final List<String> inputTables = join.getInputs().stream()
-            .filter(input -> input instanceof TableScan)
-            .map(scan -> Util.last(scan.getTable().getQualifiedName()))
-            .collect(Collectors.toList());
+        final List<String> inputTables = new TableNameVisitor().run(rel);
         return equalsStringList(tableNames, inputTables);
       };
     }
 
+    /**
+     * Implementation of RelVisitor to extract input table names.
+     */
+    private static class TableNameVisitor extends RelVisitor {
+      private List<String> names = new ArrayList<>();
+
+      List<String> run(RelNode input) {
+        go(input);
+        return names;
+      }
+
+      @Override public void visit(RelNode node, int ordinal, @Nullable RelNode parent) {
+        if (node instanceof TableScan) {
+          RelOptTable table = node.getTable();
+          String tableName = Util.last(table.getQualifiedName());

Review comment:
       Hi danny, I updated the logical to extract table names. Please have a look at the pr again, thanks a lot.




-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org