jamesstarr commented on a change in pull request #2638:
URL: https://github.com/apache/calcite/pull/2638#discussion_r766384348



##########
File path: 
core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java
##########
@@ -75,11 +78,20 @@
       new ThreadLocal<>();
 
   //~ Constructors -----------------------------------------------------------
-
+  @Deprecated // to be removed before 2.0
   protected RelMetadataQueryBase(@Nullable JaninoRelMetadataProvider 
metadataProvider) {
-    this.metadataProvider = metadataProvider;
+    this((MetadataHandlerProvider) metadataProvider);
+  }
+
+  @SuppressWarnings("deprecation")
+  protected RelMetadataQueryBase(@Nullable MetadataHandlerProvider provider) {
+    this.provider = provider;
+    this.metadataProvider = provider instanceof JaninoRelMetadataProvider
+        ? (JaninoRelMetadataProvider) provider : null;
   }
 
+  @SuppressWarnings("deprecation")

Review comment:
       You should not need this suppressWarning.

##########
File path: 
core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java
##########
@@ -67,6 +67,9 @@
   /** Set of active metadata queries, and cache of previous results. */
   public final Table<RelNode, Object, Object> map = HashBasedTable.create();
 
+  protected final @Nullable MetadataHandlerProvider provider;

Review comment:
       I think a slightly more verbose name would called for.  At least 
handlerProvider if not the monty of metadataHandlerProvider.

##########
File path: 
core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQueryBase.java
##########
@@ -75,11 +78,20 @@
       new ThreadLocal<>();
 
   //~ Constructors -----------------------------------------------------------
-
+  @Deprecated // to be removed before 2.0
   protected RelMetadataQueryBase(@Nullable JaninoRelMetadataProvider 
metadataProvider) {

Review comment:
       This can be removed.  No downstream project will directly call this.

##########
File path: 
core/src/main/java/org/apache/calcite/rel/metadata/RelMetadataQuery.java
##########
@@ -145,9 +184,10 @@ private RelMetadataQuery(@SuppressWarnings("unused") 
boolean dummy) {
     this.lowerBoundCostHandler = 
initialHandler(BuiltInMetadata.LowerBoundCost.Handler.class);
   }
 
-  private RelMetadataQuery(JaninoRelMetadataProvider metadataProvider,
+  private RelMetadataQuery(
+      @Nullable MetadataHandlerProvider metadataHandlerProvider,

Review comment:
       Why is this nullable?




-- 
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-unsubscr...@calcite.apache.org

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


Reply via email to