mihaibudiu commented on code in PR #5059:
URL: https://github.com/apache/calcite/pull/5059#discussion_r3501990735


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImplementorTable.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.adapter.enumerable;
+
+import org.apache.calcite.adapter.enumerable.RexImpTable.RexCallImplementor;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlMatchFunction;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlWindowTableFunction;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+/**
+ * Resolves the code-generation implementor for an operator, aggregate, match,
+ * or table function.
+ *
+ * <p>This is the code-generation counterpart of {@link 
org.apache.calcite.sql.SqlOperatorTable}:

Review Comment:
   I would not mention SqlOperatorTable, just describe what this interface does.



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImplementorTable.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.adapter.enumerable;
+
+import org.apache.calcite.adapter.enumerable.RexImpTable.RexCallImplementor;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlMatchFunction;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlWindowTableFunction;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+/**
+ * Resolves the code-generation implementor for an operator, aggregate, match,

Review Comment:
   implementor or implementors? There seem to be one of each kind.



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##########
@@ -191,11 +195,22 @@ private RexToLixTranslator(@Nullable RexProgram program,
    *                   variables
    * @return Sequence of expressions, optional condition
    */
+  @Deprecated // to be removed before 2.0

Review Comment:
   Add also a `@deprecated` annotation and point to the right API to use 
instead.



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImplementorTable.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.adapter.enumerable;
+
+import org.apache.calcite.adapter.enumerable.RexImpTable.RexCallImplementor;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlMatchFunction;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlWindowTableFunction;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+/**
+ * Resolves the code-generation implementor for an operator, aggregate, match,
+ * or table function.
+ *
+ * <p>This is the code-generation counterpart of {@link 
org.apache.calcite.sql.SqlOperatorTable}:
+ * where an operator table maps names to operators during validation, a
+ * {@code RexImplementorTable} maps operators to the implementors that 
translate
+ * their calls to {@linkplain org.apache.calcite.linq4j.tree.Expression linq4j 
code}
+ * during {@link org.apache.calcite.adapter.enumerable.EnumerableConvention 
enumerable}
+ * code generation.
+ *
+ * <p>The built-in implementations live in {@link RexImpTable}

Review Comment:
   I think this paragraph only belongs in chain method, not here. It is not the 
job of an interface to describe who is implementing it and how.



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##########
@@ -1218,32 +1236,43 @@ private Expression translateTableFunction(RexCall 
rexCall, Expression inputEnume
       PhysType inputPhysType, PhysType outputPhysType) {
     assert rexCall.getOperator() instanceof SqlWindowTableFunction;
     TableFunctionCallImplementor implementor =
-        RexImpTable.INSTANCE.get((SqlWindowTableFunction) 
rexCall.getOperator());
+        implementorTable.get((SqlWindowTableFunction) rexCall.getOperator());
     if (implementor == null) {
       throw Util.needToImplement("implementor of " + 
rexCall.getOperator().getName());
     }
     return implementor.implement(
         this, inputEnumerable, rexCall, inputPhysType, outputPhysType);
   }
 
+  @Deprecated // to be removed before 2.0
   public static Expression translateCondition(RexProgram program,
       JavaTypeFactory typeFactory, BlockBuilder list, InputGetter inputGetter,
       Function1<String, InputGetter> correlates, SqlConformance conformance) {
     return translateCondition(program, typeFactory, list, inputGetter,
-        correlates, conformance, false);
+        correlates, conformance, false, RexImpTable.INSTANCE);
   }
 
+  @Deprecated // to be removed before 2.0

Review Comment:
   see other comment above



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImplementorTables.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * 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.adapter.enumerable;
+
+import org.apache.calcite.adapter.enumerable.RexImpTable.RexCallImplementor;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlMatchFunction;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlWindowTableFunction;
+
+import com.google.common.collect.ImmutableList;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Utilities for {@link RexImplementorTable}.
+ *
+ * <p>The code-generation counterpart of

Review Comment:
   This comment is a bit too abstract, and maybe not needed, the interface has 
a better comment.



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