lincoln-lil commented on code in PR #24890:
URL: https://github.com/apache/flink/pull/24890#discussion_r1630981891
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/FunctionITCase.java:
##########
@@ -1542,6 +1542,22 @@ void testUsingAddJar() throws Exception {
"drop function lowerUdf");
}
+ @Test
+ void testUdfWithMultiLocalVariables() {
+ List<Row> sourceData = Arrays.asList(Row.of(1L, 2L), Row.of(2L, 3L));
+ TestCollectionTableFactory.reset();
+ TestCollectionTableFactory.initData(sourceData);
+
+ tEnv().executeSql(
+ "CREATE TABLE SourceTable(x BIGINT, y BIGINT) WITH
('connector' = 'COLLECTION')");
+ tEnv().executeSql(
+ "CREATE FUNCTION MultiLocalVariables AS '"
+ + MultiLocalVariableBlocksClass.class.getName()
+ + "'");
+ CollectionUtil.iteratorToList(
+ tEnv().executeSql("SELECT MultiLocalVariables(x, y) FROM
SourceTable").collect());
Review Comment:
We should add the verification for the running result since it is an IT
case, otherwise we can add plan test for this.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/ExtractionUtils.java:
##########
@@ -844,7 +846,14 @@ private static class ParameterExtractor extends
ClassVisitor {
}
List<String> getParameterNames() {
- return parameterNames;
+ // method parameters are always at the head in the 'index' list
Review Comment:
It is recommended move this description toi the class comments, and also
update the example to illustrate the need to sort by index.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/ExtractionUtils.java:
##########
@@ -844,7 +846,14 @@ private static class ParameterExtractor extends
ClassVisitor {
}
List<String> getParameterNames() {
- return parameterNames;
+ // method parameters are always at the head in the 'index' list
+ // NOTE: the first parameter may be "this" if the function is not
static
+ // See more at Chapter "3.6. Receiving Arguments" in
+ // https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-3.html
+ return parameterNamesWithIndex.entrySet().stream()
Review Comment:
We can use `TreeMap` instead of `HashMap` for `parameterNamesWithIndex` to
avoid sorting operation and the use of java streams, WDYT?
--
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]
