[calcite] branch master updated: [CALCITE-3487] Should not hard code RelMetadataQuery class in VolcanoPlanner.isValid() (Xiening Dai)

[hy...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
     new f90e5d7  [CALCITE-3487] Should not hard code RelMetadataQuery class in VolcanoPlanner.isValid() (Xiening Dai)
f90e5d7 is described below

commit f90e5d7b48848191c1a70e6c0f78eece93433806
Author: Xiening Dai <xn...@live.com>
AuthorDate: Fri Nov 8 10:05:49 2019 -0800

    [CALCITE-3487] Should not hard code RelMetadataQuery class in VolcanoPlanner.isValid() (Xiening Dai)
    
    Add a new method to get RelMetadataQuery supplier from RelOptCluster. And use
    the supplier to create corresponding RelMetadataQuery object in isValid() call.
    
    Also rename setMetadataQuery() to setMetadataQuerySupplier().
    
    Close #1569
---
 .../main/java/org/apache/calcite/plan/RelOptCluster.java  | 15 ++++++++++++---
 .../org/apache/calcite/plan/volcano/VolcanoPlanner.java   |  2 +-
 .../apache/calcite/rel/metadata/RelMetadataQueryBase.java |  2 +-
 .../java/org/apache/calcite/test/RelMetadataTest.java     |  4 ++--
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptCluster.java b/core/src/main/java/org/apache/calcite/plan/RelOptCluster.java
index 88d0911..79dcc5c 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptCluster.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptCluster.java
@@ -87,7 +87,7 @@ public class RelOptCluster {
     // set up a default rel metadata provider,
     // giving the planner first crack at everything
     setMetadataProvider(DefaultRelMetadataProvider.INSTANCE);
-    setMetadataQuery(RelMetadataQuery::instance);
+    setMetadataQuerySupplier(RelMetadataQuery::instance);
     this.emptyTraitSet = planner.emptyTraitSet();
     assert emptyTraitSet.size() == planner.getRelTraitDefs().size();
   }
@@ -152,14 +152,15 @@ public class RelOptCluster {
   }
 
   /**
-   * Set up the customized {@link RelMetadataQuery} instance that to use during rule planning.
+   * Set up the customized {@link RelMetadataQuery} instance supplier that to
+   * use during rule planning.
    *
    * <p>Note that the {@code mqSupplier} should return
    * a fresh new {@link RelMetadataQuery} instance because the instance would be
    * cached in this cluster, and we may invalidate and re-generate it
    * for each {@link RelOptRuleCall} cycle.
    */
-  public void setMetadataQuery(Supplier<RelMetadataQuery> mqSupplier) {
+  public void setMetadataQuerySupplier(Supplier<RelMetadataQuery> mqSupplier) {
     this.mqSupplier = mqSupplier;
   }
 
@@ -178,6 +179,14 @@ public class RelOptCluster {
   }
 
   /**
+   * Returns the RelMetadataQuery supplier
+   * @return The supplier of RelMetadataQuery
+   */
+  public Supplier<RelMetadataQuery> getMetadataQuerySupplier() {
+    return this.mqSupplier;
+  }
+
+  /**
    * Should be called whenever the current {@link RelMetadataQuery} becomes
    * invalid. Typically invoked from {@link RelOptRuleCall#transformTo}.
    */
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
index d28b5ce..9901ff3 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
@@ -877,7 +877,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
    * Checks internal consistency.
    */
   protected boolean isValid(Litmus litmus) {
-    RelMetadataQuery metaQuery = RelMetadataQuery.instance();
+    RelMetadataQuery metaQuery = this.getRoot().getCluster().getMetadataQuerySupplier().get();
     for (RelSet set : allSets) {
       if (set.equivalentSet != null) {
         return litmus.fail("set [{}] has been merged: it should not be in the list", set);
diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java
index aff4fef..3fbd935 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java
@@ -48,7 +48,7 @@ import java.util.function.Supplier;
  * "SOURCE"s with default ones also works).
  * <li>Set {@code MyRelMetadataProvider} into the cluster instance.
  * <li>Use
- * {@link org.apache.calcite.plan.RelOptCluster#setMetadataQuery(Supplier)}
+ * {@link org.apache.calcite.plan.RelOptCluster#setMetadataQuerySupplier(Supplier)}
  * to set the metadata query {@link Supplier} into the cluster instance. This {@link Supplier}
  * should return a <strong>fresh new</strong> instance.
  * <li>Use the cluster instance to create
diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
index 8197543..39b5ea6 100644
--- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
@@ -1172,7 +1172,7 @@ public class RelMetadataTest extends SqlToRelTestBase {
               ChainedRelMetadataProvider.of(
                   ImmutableList.of(BrokenColTypeImpl.SOURCE,
                       cluster.getMetadataProvider())));
-          cluster.setMetadataQuery(MyRelMetadataQuery::new);
+          cluster.setMetadataQuerySupplier(MyRelMetadataQuery::new);
           return cluster;
         })
         .convertSqlToRel(sql);
@@ -1282,7 +1282,7 @@ public class RelMetadataTest extends SqlToRelTestBase {
                   cluster.getMetadataProvider());
           cluster.setMetadataProvider(
               ChainedRelMetadataProvider.of(list));
-          cluster.setMetadataQuery(MyRelMetadataQuery::new);
+          cluster.setMetadataQuerySupplier(MyRelMetadataQuery::new);
           return cluster;
         })
         .convertSqlToRel(sql);