You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2021/11/10 01:12:05 UTC

[GitHub] [calcite] jacques-n opened a new pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

jacques-n opened a new pull request #2603:
URL: https://github.com/apache/calcite/pull/2603


   - Introduce a Lambda-based RelMetadataQuery creator for AOT environments
   - Add new microbenchmark for metadata retrieval on a five-way-join
   - Fix incorrect results for disabled test JdbcTest.testJoinFiveWay()
   - Update jmh-gradle-plugin version.


-- 
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



[GitHub] [calcite] jacques-n edited a comment on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n edited a comment on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-964689308


   Working to update RelMdTest to be parameterized. (I've run the test locally with a hardcode change to use the alternative metadata supplier.)


-- 
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



[GitHub] [calcite] jacques-n commented on a change in pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#discussion_r747023794



##########
File path: core/src/main/java/org/apache/calcite/rel/metadata/lambda/ReflectionToLambdaProvider.java
##########
@@ -0,0 +1,359 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.lambda;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.metadata.RelMdAllPredicates;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMdColumnOrigins;
+import org.apache.calcite.rel.metadata.RelMdColumnUniqueness;
+import org.apache.calcite.rel.metadata.RelMdDistinctRowCount;
+import org.apache.calcite.rel.metadata.RelMdDistribution;
+import org.apache.calcite.rel.metadata.RelMdExplainVisibility;
+import org.apache.calcite.rel.metadata.RelMdExpressionLineage;
+import org.apache.calcite.rel.metadata.RelMdLowerBoundCost;
+import org.apache.calcite.rel.metadata.RelMdMaxRowCount;
+import org.apache.calcite.rel.metadata.RelMdMemory;
+import org.apache.calcite.rel.metadata.RelMdMinRowCount;
+import org.apache.calcite.rel.metadata.RelMdNodeTypes;
+import org.apache.calcite.rel.metadata.RelMdParallelism;
+import org.apache.calcite.rel.metadata.RelMdPercentageOriginalRows;
+import org.apache.calcite.rel.metadata.RelMdPopulationSize;
+import org.apache.calcite.rel.metadata.RelMdPredicates;
+import org.apache.calcite.rel.metadata.RelMdRowCount;
+import org.apache.calcite.rel.metadata.RelMdSelectivity;
+import org.apache.calcite.rel.metadata.RelMdSize;
+import org.apache.calcite.rel.metadata.RelMdTableReferences;
+import org.apache.calcite.rel.metadata.RelMdUniqueKeys;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.BuiltInMethod;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Table;
+import com.google.common.primitives.Primitives;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.LambdaMetafactory;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * Create a set of lambda handlers via reflection patterns.
+ *
+ * This does reflection to lambda conversion based on a set of singleton objects
+ * with appropriate signatures, similar to how ReflectiveRelMetadataProvider works.
+ * Any class that can be discovered using ReflectiveRelMetadataProvider should also be consumable
+ * using this class. Ultimately, the goal may be to move to direct lambda registration as opposed to
+ * the old system of partial reflection discovery.
+ */
+@ThreadSafe
+public class ReflectionToLambdaProvider implements LambdaProvider {
+
+  public static final ImmutableList<Source> DEFAULT_SOURCES = ImmutableList.<Source>builder()

Review comment:
       This is effectively a reimagining of `MetadataHandler` as a functional interface with a more useful hierarchy/generic definition. Could we convert MetadataHandler to provide this functionality: yes, we probably could. 
   
   I haven't spent time trying to untangle the way we would get there. I would expect that it would take many releases to try minimize disruption while slowly converting all the interfaces/impls/etc.
   
   
   




-- 
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



[GitHub] [calcite] jamesstarr commented on a change in pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jamesstarr commented on a change in pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#discussion_r747073723



##########
File path: core/src/main/java/org/apache/calcite/rel/metadata/lambda/ReflectionToLambdaProvider.java
##########
@@ -0,0 +1,359 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.lambda;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.metadata.RelMdAllPredicates;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMdColumnOrigins;
+import org.apache.calcite.rel.metadata.RelMdColumnUniqueness;
+import org.apache.calcite.rel.metadata.RelMdDistinctRowCount;
+import org.apache.calcite.rel.metadata.RelMdDistribution;
+import org.apache.calcite.rel.metadata.RelMdExplainVisibility;
+import org.apache.calcite.rel.metadata.RelMdExpressionLineage;
+import org.apache.calcite.rel.metadata.RelMdLowerBoundCost;
+import org.apache.calcite.rel.metadata.RelMdMaxRowCount;
+import org.apache.calcite.rel.metadata.RelMdMemory;
+import org.apache.calcite.rel.metadata.RelMdMinRowCount;
+import org.apache.calcite.rel.metadata.RelMdNodeTypes;
+import org.apache.calcite.rel.metadata.RelMdParallelism;
+import org.apache.calcite.rel.metadata.RelMdPercentageOriginalRows;
+import org.apache.calcite.rel.metadata.RelMdPopulationSize;
+import org.apache.calcite.rel.metadata.RelMdPredicates;
+import org.apache.calcite.rel.metadata.RelMdRowCount;
+import org.apache.calcite.rel.metadata.RelMdSelectivity;
+import org.apache.calcite.rel.metadata.RelMdSize;
+import org.apache.calcite.rel.metadata.RelMdTableReferences;
+import org.apache.calcite.rel.metadata.RelMdUniqueKeys;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.BuiltInMethod;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Table;
+import com.google.common.primitives.Primitives;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.LambdaMetafactory;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * Create a set of lambda handlers via reflection patterns.
+ *
+ * This does reflection to lambda conversion based on a set of singleton objects
+ * with appropriate signatures, similar to how ReflectiveRelMetadataProvider works.
+ * Any class that can be discovered using ReflectiveRelMetadataProvider should also be consumable
+ * using this class. Ultimately, the goal may be to move to direct lambda registration as opposed to
+ * the old system of partial reflection discovery.
+ */
+@ThreadSafe
+public class ReflectionToLambdaProvider implements LambdaProvider {
+
+  public static final ImmutableList<Source> DEFAULT_SOURCES = ImmutableList.<Source>builder()

Review comment:
       The Lambda API is leaking the dispatch implementation into the API for binding in custom implementations.  These APIs should be separate.  It is actually fairly easy to adept this approach to use handlers and not be distributive to down stream projects. 




-- 
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



[GitHub] [calcite] jacques-n edited a comment on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n edited a comment on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-964689308


   Working to update `RelMetadataTest` to be parameterized. (I've run the test locally with a hardcoded change to use the alternative metadata supplier.)


-- 
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



[GitHub] [calcite] jacques-n commented on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n commented on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-1058521354


   > I'm curious, what is the reason why this PR is blocked?
   
   I was waiting for CALCITE-4942 to be resolved since it could potentially substantially reduce the amount of boilerplate and reimplementation in this patch. Note that I did get [ProxyingMetadataHandlerProvider|https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/metadata/ProxyingMetadataHandlerProvider.java] which allows one to use metadata without runtime compilation. It isn't fast but at least it is functional in things like GraalVM.


-- 
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



[GitHub] [calcite] jamesstarr commented on a change in pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jamesstarr commented on a change in pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#discussion_r746991256



##########
File path: core/src/main/java/org/apache/calcite/rel/metadata/lambda/ReflectionToLambdaProvider.java
##########
@@ -0,0 +1,359 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.lambda;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.metadata.RelMdAllPredicates;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMdColumnOrigins;
+import org.apache.calcite.rel.metadata.RelMdColumnUniqueness;
+import org.apache.calcite.rel.metadata.RelMdDistinctRowCount;
+import org.apache.calcite.rel.metadata.RelMdDistribution;
+import org.apache.calcite.rel.metadata.RelMdExplainVisibility;
+import org.apache.calcite.rel.metadata.RelMdExpressionLineage;
+import org.apache.calcite.rel.metadata.RelMdLowerBoundCost;
+import org.apache.calcite.rel.metadata.RelMdMaxRowCount;
+import org.apache.calcite.rel.metadata.RelMdMemory;
+import org.apache.calcite.rel.metadata.RelMdMinRowCount;
+import org.apache.calcite.rel.metadata.RelMdNodeTypes;
+import org.apache.calcite.rel.metadata.RelMdParallelism;
+import org.apache.calcite.rel.metadata.RelMdPercentageOriginalRows;
+import org.apache.calcite.rel.metadata.RelMdPopulationSize;
+import org.apache.calcite.rel.metadata.RelMdPredicates;
+import org.apache.calcite.rel.metadata.RelMdRowCount;
+import org.apache.calcite.rel.metadata.RelMdSelectivity;
+import org.apache.calcite.rel.metadata.RelMdSize;
+import org.apache.calcite.rel.metadata.RelMdTableReferences;
+import org.apache.calcite.rel.metadata.RelMdUniqueKeys;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.BuiltInMethod;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Table;
+import com.google.common.primitives.Primitives;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.LambdaMetafactory;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * Create a set of lambda handlers via reflection patterns.
+ *
+ * This does reflection to lambda conversion based on a set of singleton objects
+ * with appropriate signatures, similar to how ReflectiveRelMetadataProvider works.
+ * Any class that can be discovered using ReflectiveRelMetadataProvider should also be consumable
+ * using this class. Ultimately, the goal may be to move to direct lambda registration as opposed to
+ * the old system of partial reflection discovery.
+ */
+@ThreadSafe
+public class ReflectionToLambdaProvider implements LambdaProvider {
+
+  public static final ImmutableList<Source> DEFAULT_SOURCES = ImmutableList.<Source>builder()

Review comment:
       If all handlers were refactored to only have 1 methods, couldn't we drive this off RelMetadataProvider, if a visiter pattern was exposed?




-- 
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



[GitHub] [calcite] jacques-n commented on a change in pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#discussion_r747112513



##########
File path: core/src/main/java/org/apache/calcite/rel/metadata/lambda/ReflectionToLambdaProvider.java
##########
@@ -0,0 +1,359 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.lambda;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.metadata.RelMdAllPredicates;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMdColumnOrigins;
+import org.apache.calcite.rel.metadata.RelMdColumnUniqueness;
+import org.apache.calcite.rel.metadata.RelMdDistinctRowCount;
+import org.apache.calcite.rel.metadata.RelMdDistribution;
+import org.apache.calcite.rel.metadata.RelMdExplainVisibility;
+import org.apache.calcite.rel.metadata.RelMdExpressionLineage;
+import org.apache.calcite.rel.metadata.RelMdLowerBoundCost;
+import org.apache.calcite.rel.metadata.RelMdMaxRowCount;
+import org.apache.calcite.rel.metadata.RelMdMemory;
+import org.apache.calcite.rel.metadata.RelMdMinRowCount;
+import org.apache.calcite.rel.metadata.RelMdNodeTypes;
+import org.apache.calcite.rel.metadata.RelMdParallelism;
+import org.apache.calcite.rel.metadata.RelMdPercentageOriginalRows;
+import org.apache.calcite.rel.metadata.RelMdPopulationSize;
+import org.apache.calcite.rel.metadata.RelMdPredicates;
+import org.apache.calcite.rel.metadata.RelMdRowCount;
+import org.apache.calcite.rel.metadata.RelMdSelectivity;
+import org.apache.calcite.rel.metadata.RelMdSize;
+import org.apache.calcite.rel.metadata.RelMdTableReferences;
+import org.apache.calcite.rel.metadata.RelMdUniqueKeys;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.BuiltInMethod;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Table;
+import com.google.common.primitives.Primitives;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.LambdaMetafactory;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * Create a set of lambda handlers via reflection patterns.
+ *
+ * This does reflection to lambda conversion based on a set of singleton objects
+ * with appropriate signatures, similar to how ReflectiveRelMetadataProvider works.
+ * Any class that can be discovered using ReflectiveRelMetadataProvider should also be consumable
+ * using this class. Ultimately, the goal may be to move to direct lambda registration as opposed to
+ * the old system of partial reflection discovery.
+ */
+@ThreadSafe
+public class ReflectionToLambdaProvider implements LambdaProvider {
+
+  public static final ImmutableList<Source> DEFAULT_SOURCES = ImmutableList.<Source>builder()

Review comment:
       Can you expound on your comments. They are a bit brief. 
   
   >Lambda API is leaking the dispatch implementation into the API for binding in custom implementations
   Please be more specific about what exactly you think is leaking where. I think we may have different perspectives of what functionality is trying to be provided and how. The design here is that the declaration of a specific MetadataLambda implementation gives all the information about what RelNode types it applies to and what metadata it provides. I haven't yet posted the simple provider that simply returns hardcoded lambdas, but that was part of the goal for this design.
   
   > It is actually fairly easy to adept this approach to use handlers
   I assume you mean adapt. I can see how one could make MetadataLambdas pretend to be handlers with a bunch of boilerplate but I don't see why that is a good idea. What exactly would this buy? Using janino on top of lambdas?
   
   > and not be distributive to down stream projects.
   I assume you mean disruptive? I don't see how any of these changes are disruptive. This is just an alternative way to create RelMetadataQuery instances and specify metadata (via lambdas).
   
   FYI, I will be updating this patch that to also include my vision for how to handle custom metadata types without any reflection and without excessive subclassing.




-- 
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



[GitHub] [calcite] jacques-n commented on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n commented on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-977206223


   > Just wondering, does it have something in common with https://github.com/forax/exotic#structuralcall---javadoc ?
   
   Similar, but not the same. Here we're using LambdaMetafactory and actually convert the function call to a generic version. Core of that work is [here](https://github.com/apache/calcite/blob/b69ffa2c0eeb65b1bb5c46d745730cdace4d5e4c/core/src/main/java/org/apache/calcite/rel/metadata/lambda/ReflectionToLambdaProvider.java#L260).


-- 
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



[GitHub] [calcite] jacques-n edited a comment on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n edited a comment on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-964689308


   Working to update RelMdTest to be parameterized. (I've run the test locally with a hardcoded change to use the alternative metadata supplier.)


-- 
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



[GitHub] [calcite] jacques-n commented on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n commented on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-964723151


   Added reuse of RelMetdataTest. Will work on null checker next...


-- 
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



[GitHub] [calcite] jamesstarr commented on a change in pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jamesstarr commented on a change in pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#discussion_r747745008



##########
File path: core/src/main/java/org/apache/calcite/rel/metadata/lambda/ReflectionToLambdaProvider.java
##########
@@ -0,0 +1,359 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.lambda;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.metadata.RelMdAllPredicates;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMdColumnOrigins;
+import org.apache.calcite.rel.metadata.RelMdColumnUniqueness;
+import org.apache.calcite.rel.metadata.RelMdDistinctRowCount;
+import org.apache.calcite.rel.metadata.RelMdDistribution;
+import org.apache.calcite.rel.metadata.RelMdExplainVisibility;
+import org.apache.calcite.rel.metadata.RelMdExpressionLineage;
+import org.apache.calcite.rel.metadata.RelMdLowerBoundCost;
+import org.apache.calcite.rel.metadata.RelMdMaxRowCount;
+import org.apache.calcite.rel.metadata.RelMdMemory;
+import org.apache.calcite.rel.metadata.RelMdMinRowCount;
+import org.apache.calcite.rel.metadata.RelMdNodeTypes;
+import org.apache.calcite.rel.metadata.RelMdParallelism;
+import org.apache.calcite.rel.metadata.RelMdPercentageOriginalRows;
+import org.apache.calcite.rel.metadata.RelMdPopulationSize;
+import org.apache.calcite.rel.metadata.RelMdPredicates;
+import org.apache.calcite.rel.metadata.RelMdRowCount;
+import org.apache.calcite.rel.metadata.RelMdSelectivity;
+import org.apache.calcite.rel.metadata.RelMdSize;
+import org.apache.calcite.rel.metadata.RelMdTableReferences;
+import org.apache.calcite.rel.metadata.RelMdUniqueKeys;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.BuiltInMethod;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Table;
+import com.google.common.primitives.Primitives;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.LambdaMetafactory;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * Create a set of lambda handlers via reflection patterns.
+ *
+ * This does reflection to lambda conversion based on a set of singleton objects
+ * with appropriate signatures, similar to how ReflectiveRelMetadataProvider works.
+ * Any class that can be discovered using ReflectiveRelMetadataProvider should also be consumable
+ * using this class. Ultimately, the goal may be to move to direct lambda registration as opposed to
+ * the old system of partial reflection discovery.
+ */
+@ThreadSafe
+public class ReflectionToLambdaProvider implements LambdaProvider {
+
+  public static final ImmutableList<Source> DEFAULT_SOURCES = ImmutableList.<Source>builder()

Review comment:
       If this is not the purposed end api, then we should not waste too much time discussing this implementation.  But just as the API for metadata retrieval should not expose internals, the API for registering metadata implementations.  This has not been the case for the previous api.
   
   If your purposed API for this is to register every method the RelNode type instead of discovering them reflectively, then my questions is why do we need some convoluted framework for handling the dispatch instead of just harding coding in the dispatch.  Their is pretty much a one to one mapping at that point of the amount of configuration needed compare to explicitly maintaining the implementation.




-- 
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



[GitHub] [calcite] vlsi commented on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
vlsi commented on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-977190770


   Just wondering, does it have something in common with https://github.com/forax/exotic#structuralcall---javadoc ?


-- 
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



[GitHub] [calcite] linghengqian commented on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
linghengqian commented on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-1051068842


   I'm curious, what is the reason why this PR is blocked?


-- 
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



[GitHub] [calcite] jacques-n commented on a change in pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#discussion_r747031277



##########
File path: core/src/main/java/org/apache/calcite/rel/metadata/lambda/ReflectionToLambdaProvider.java
##########
@@ -0,0 +1,359 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.lambda;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.metadata.RelMdAllPredicates;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMdColumnOrigins;
+import org.apache.calcite.rel.metadata.RelMdColumnUniqueness;
+import org.apache.calcite.rel.metadata.RelMdDistinctRowCount;
+import org.apache.calcite.rel.metadata.RelMdDistribution;
+import org.apache.calcite.rel.metadata.RelMdExplainVisibility;
+import org.apache.calcite.rel.metadata.RelMdExpressionLineage;
+import org.apache.calcite.rel.metadata.RelMdLowerBoundCost;
+import org.apache.calcite.rel.metadata.RelMdMaxRowCount;
+import org.apache.calcite.rel.metadata.RelMdMemory;
+import org.apache.calcite.rel.metadata.RelMdMinRowCount;
+import org.apache.calcite.rel.metadata.RelMdNodeTypes;
+import org.apache.calcite.rel.metadata.RelMdParallelism;
+import org.apache.calcite.rel.metadata.RelMdPercentageOriginalRows;
+import org.apache.calcite.rel.metadata.RelMdPopulationSize;
+import org.apache.calcite.rel.metadata.RelMdPredicates;
+import org.apache.calcite.rel.metadata.RelMdRowCount;
+import org.apache.calcite.rel.metadata.RelMdSelectivity;
+import org.apache.calcite.rel.metadata.RelMdSize;
+import org.apache.calcite.rel.metadata.RelMdTableReferences;
+import org.apache.calcite.rel.metadata.RelMdUniqueKeys;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.BuiltInMethod;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Table;
+import com.google.common.primitives.Primitives;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.LambdaMetafactory;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * Create a set of lambda handlers via reflection patterns.
+ *
+ * This does reflection to lambda conversion based on a set of singleton objects
+ * with appropriate signatures, similar to how ReflectiveRelMetadataProvider works.
+ * Any class that can be discovered using ReflectiveRelMetadataProvider should also be consumable
+ * using this class. Ultimately, the goal may be to move to direct lambda registration as opposed to
+ * the old system of partial reflection discovery.
+ */
+@ThreadSafe
+public class ReflectionToLambdaProvider implements LambdaProvider {
+
+  public static final ImmutableList<Source> DEFAULT_SOURCES = ImmutableList.<Source>builder()

Review comment:
       I'll add, in general the whole pattern of arbitrary classes carrying arbitrary methods that name match and arguments kind-of match seems like something that wouldn't translate well to a lambda pattern (which is why the concrete RelNode argument is part of the generic interface definition in this lambda pattern).




-- 
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



[GitHub] [calcite] jamesstarr commented on a change in pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jamesstarr commented on a change in pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#discussion_r747745008



##########
File path: core/src/main/java/org/apache/calcite/rel/metadata/lambda/ReflectionToLambdaProvider.java
##########
@@ -0,0 +1,359 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.lambda;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.metadata.RelMdAllPredicates;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMdColumnOrigins;
+import org.apache.calcite.rel.metadata.RelMdColumnUniqueness;
+import org.apache.calcite.rel.metadata.RelMdDistinctRowCount;
+import org.apache.calcite.rel.metadata.RelMdDistribution;
+import org.apache.calcite.rel.metadata.RelMdExplainVisibility;
+import org.apache.calcite.rel.metadata.RelMdExpressionLineage;
+import org.apache.calcite.rel.metadata.RelMdLowerBoundCost;
+import org.apache.calcite.rel.metadata.RelMdMaxRowCount;
+import org.apache.calcite.rel.metadata.RelMdMemory;
+import org.apache.calcite.rel.metadata.RelMdMinRowCount;
+import org.apache.calcite.rel.metadata.RelMdNodeTypes;
+import org.apache.calcite.rel.metadata.RelMdParallelism;
+import org.apache.calcite.rel.metadata.RelMdPercentageOriginalRows;
+import org.apache.calcite.rel.metadata.RelMdPopulationSize;
+import org.apache.calcite.rel.metadata.RelMdPredicates;
+import org.apache.calcite.rel.metadata.RelMdRowCount;
+import org.apache.calcite.rel.metadata.RelMdSelectivity;
+import org.apache.calcite.rel.metadata.RelMdSize;
+import org.apache.calcite.rel.metadata.RelMdTableReferences;
+import org.apache.calcite.rel.metadata.RelMdUniqueKeys;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.BuiltInMethod;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Table;
+import com.google.common.primitives.Primitives;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.LambdaMetafactory;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * Create a set of lambda handlers via reflection patterns.
+ *
+ * This does reflection to lambda conversion based on a set of singleton objects
+ * with appropriate signatures, similar to how ReflectiveRelMetadataProvider works.
+ * Any class that can be discovered using ReflectiveRelMetadataProvider should also be consumable
+ * using this class. Ultimately, the goal may be to move to direct lambda registration as opposed to
+ * the old system of partial reflection discovery.
+ */
+@ThreadSafe
+public class ReflectionToLambdaProvider implements LambdaProvider {
+
+  public static final ImmutableList<Source> DEFAULT_SOURCES = ImmutableList.<Source>builder()

Review comment:
       If this is not the purposed end api, then we should not waste too much time discussing this implementation.  But just as the API for metadata retrieval should not expose internals, the API for registering metadata implementations.  This has not been the case for the previous api.
   
   If your purposed API for this is to register every method the RelNode type instead of discover theming reflectively, then my questions is why do we need some convoluted framework for handling the dispatch instead of just harding coding in the dispatch.  Their is pretty much a one to one mapping at that point of the amount of configuration needed compare to explicitly maintaining the implementation.




-- 
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



[GitHub] [calcite] jacques-n commented on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n commented on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-1060017345


   > I thought there was not agreement on how to go forward with [CALCITE-4942](https://issues.apache.org/jira/browse/CALCITE-4942)
   
   I posted a comment on the JIRA for CALCITE-4942 in late December but never saw a response (afaik).
   
   


-- 
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



[GitHub] [calcite] jacques-n commented on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n commented on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-964689308


   Working to update RelMdTest to be parameterized...


-- 
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



[GitHub] [calcite] jacques-n commented on a change in pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#discussion_r747023794



##########
File path: core/src/main/java/org/apache/calcite/rel/metadata/lambda/ReflectionToLambdaProvider.java
##########
@@ -0,0 +1,359 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.metadata.lambda;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.metadata.RelMdAllPredicates;
+import org.apache.calcite.rel.metadata.RelMdCollation;
+import org.apache.calcite.rel.metadata.RelMdColumnOrigins;
+import org.apache.calcite.rel.metadata.RelMdColumnUniqueness;
+import org.apache.calcite.rel.metadata.RelMdDistinctRowCount;
+import org.apache.calcite.rel.metadata.RelMdDistribution;
+import org.apache.calcite.rel.metadata.RelMdExplainVisibility;
+import org.apache.calcite.rel.metadata.RelMdExpressionLineage;
+import org.apache.calcite.rel.metadata.RelMdLowerBoundCost;
+import org.apache.calcite.rel.metadata.RelMdMaxRowCount;
+import org.apache.calcite.rel.metadata.RelMdMemory;
+import org.apache.calcite.rel.metadata.RelMdMinRowCount;
+import org.apache.calcite.rel.metadata.RelMdNodeTypes;
+import org.apache.calcite.rel.metadata.RelMdParallelism;
+import org.apache.calcite.rel.metadata.RelMdPercentageOriginalRows;
+import org.apache.calcite.rel.metadata.RelMdPopulationSize;
+import org.apache.calcite.rel.metadata.RelMdPredicates;
+import org.apache.calcite.rel.metadata.RelMdRowCount;
+import org.apache.calcite.rel.metadata.RelMdSelectivity;
+import org.apache.calcite.rel.metadata.RelMdSize;
+import org.apache.calcite.rel.metadata.RelMdTableReferences;
+import org.apache.calcite.rel.metadata.RelMdUniqueKeys;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.BuiltInMethod;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Table;
+import com.google.common.primitives.Primitives;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.LambdaMetafactory;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
+import javax.annotation.concurrent.ThreadSafe;
+
+/**
+ * Create a set of lambda handlers via reflection patterns.
+ *
+ * This does reflection to lambda conversion based on a set of singleton objects
+ * with appropriate signatures, similar to how ReflectiveRelMetadataProvider works.
+ * Any class that can be discovered using ReflectiveRelMetadataProvider should also be consumable
+ * using this class. Ultimately, the goal may be to move to direct lambda registration as opposed to
+ * the old system of partial reflection discovery.
+ */
+@ThreadSafe
+public class ReflectionToLambdaProvider implements LambdaProvider {
+
+  public static final ImmutableList<Source> DEFAULT_SOURCES = ImmutableList.<Source>builder()

Review comment:
       This is effectively a reimagining of `MetadataHandler` as a functional interface with a more useful hierarchy/generic definition. Could we convert MetdataHandler to provide this functionality: yes, we probably could. 
   
   I haven't spent time trying to untangle the way we would get there. I would expect that it would take many releases to try minimize disruption while slowly converting all the interfaces/impls/etc.
   
   
   




-- 
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



[GitHub] [calcite] jamesstarr commented on pull request #2603: [CALCITE-4882] Introduce new Lambda-based Metadata framework

Posted by GitBox <gi...@apache.org>.
jamesstarr commented on pull request #2603:
URL: https://github.com/apache/calcite/pull/2603#issuecomment-1058604078


   > > I'm curious, what is the reason why this PR is blocked?
   > 
   > I was waiting for [CALCITE-4942](https://issues.apache.org/jira/browse/CALCITE-4942) to be resolved since it could potentially substantially reduce the amount of boilerplate and reimplementation in this patch. Note that I did get [ProxyingMetadataHandlerProvider|https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/metadata/ProxyingMetadataHandlerProvider.java] which allows one to use metadata without runtime compilation. It isn't fast but at least it is functional in things like GraalVM.
   
   I thought there was not agreement on how to go forward with CALCITE-4942.  From what I recall their was disagreement on wether to expose the lambda implementation classes or to use the  ProxyingMetadataHandlerProvider to bridge interfaces between the metadata query and the handler generator.  


-- 
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