matriv commented on code in PR #19410:
URL: https://github.com/apache/flink/pull/19410#discussion_r846285936
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/FunctionCatalog.java:
##########
@@ -621,7 +626,7 @@ public void registerTemporarySystemFunction(
}
@SuppressWarnings("unchecked")
- private void validateAndPrepareFunction(CatalogFunction function)
+ private CatalogFunction validateAndPrepareFunction(CatalogFunction
function)
Review Comment:
Just a double check, have you checked all usages to make sure we use the
returned value?
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/utils/PlannerMocks.java:
##########
@@ -96,6 +102,7 @@ private PlannerMocks(
true,
ExpressionResolver.resolverFor(
tableConfig,
+ Thread.currentThread().getContextClassLoader(),
Review Comment:
Why not use `PlannerMocks.class.getClassLoader()` here?
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/EncodingUtils.java:
##########
@@ -139,6 +139,8 @@ public static String encodeObjectToString(Serializable obj)
{
}
}
+ /** @see #loadClass(String, ClassLoader) */
Review Comment:
please add `@deprecated`
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/catalog/CatalogFunction.java:
##########
@@ -59,6 +59,7 @@
*
* @return whether the function is a generic function
*/
+ @Deprecated
Review Comment:
Please add `@deprecated` section to the javadoc
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/plan/rules/physical/stream/StreamPhysicalJoinRuleBase.scala:
##########
@@ -25,6 +25,11 @@ import
org.apache.flink.table.planner.plan.nodes.{FlinkConventions, FlinkRelNode
import org.apache.flink.table.planner.plan.utils.IntervalJoinUtil
import org.apache.flink.table.planner.utils.ShortcutUtils.unwrapTableConfig
+import org.apache.flink.table.planner.plan.utils.{FlinkRelOptUtil,
IntervalJoinUtil}
Review Comment:
leftover?
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/UserDefinedFunctionHelper.java:
##########
@@ -244,17 +245,23 @@ public static UserDefinedFunction
instantiateFunction(Class<?> functionClass) {
}
/** Prepares a {@link UserDefinedFunction} instance for usage in the API.
*/
- public static void prepareInstance(ReadableConfig config,
UserDefinedFunction function) {
+ public static <T extends UserDefinedFunction> T prepareInstance(
Review Comment:
Maybe add to the javadoc, that the caller needs to use the returned value?
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
