You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "hackergin (via GitHub)" <gi...@apache.org> on 2024/01/16 18:34:48 UTC

[PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

hackergin opened a new pull request, #24106:
URL: https://github.com/apache/flink/pull/24106

   <!--
   *Thank you very much for contributing to Apache Flink - we are happy that you want to help us improve Flink. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.*
   
   *Please understand that we do not do this to make contributions to Flink a hassle. In order to uphold a high standard of quality for code contributions, while at the same time managing a large number of contributions, we need contributors to prepare the contributions well, and give reviewers enough contextual information for the review. Please also understand that contributions that do not follow this guide will take longer to review and thus typically be picked up with lower priority by the community.*
   
   ## Contribution Checklist
   
     - Make sure that the pull request corresponds to a [JIRA issue](https://issues.apache.org/jira/projects/FLINK/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue.
     
     - Name the pull request in the form "[FLINK-XXXX] [component] Title of the pull request", where *FLINK-XXXX* should be replaced by the actual issue number. Skip *component* if you are unsure about which is the best component.
     Typo fixes that have no associated JIRA issue should be named following this pattern: `[hotfix] [docs] Fix typo in event time introduction` or `[hotfix] [javadocs] Expand JavaDoc for PuncuatedWatermarkGenerator`.
   
     - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
     
     - Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes. You can set up Azure Pipelines CI to do that following [this guide](https://cwiki.apache.org/confluence/display/FLINK/Azure+Pipelines#AzurePipelines-Tutorial:SettingupAzurePipelinesforaforkoftheFlinkrepository).
   
     - Each pull request should address only one issue, not mix up code from multiple issues.
     
     - Each commit in the pull request has a meaningful commit message (including the JIRA id)
   
     - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
   
   
   **(The sections below can be removed for hotfixes of typos)**
   -->
   
   ## What is the purpose of the change
   
   *1. Declare a new ArgumentHint to specify the name, type, and whether it is optional for an argument.
   *2. Support named parameters for functions.
   *3. Support named parameters for procedures.
   
   
   ## Brief change log
   
     - ntroduce a new ArgumentHint annotation
     - Support named parameters for functions 
     - Support named parameters for producers
   
   
   ## Verifying this change
   
   test cases in FunctionITCase and ProcedureITCase to verify named parameters.
   
   
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
     - The serializers: (no)
     - The runtime per-record code paths (performance sensitive): (no)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
     - The S3 file system connector: (no)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (yes)
     - If yes, how is the feature documented? (not documented, will be added in a separator pr)
   


-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1464193652


##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -427,8 +428,7 @@ private static Stream<TestSpec> functionSpecs() {
                                 "Could not find a publicly accessible method named 'eval'."),
 
                 // named arguments with overloaded function
-                TestSpec.forScalarFunction(NamedArgumentsScalarFunction.class)
-                        .expectNamedArguments("n"),
+                TestSpec.forScalarFunction(NamedArgumentsScalarFunction.class),

Review Comment:
   I have added relevant comments. We expect namedArgument to be null, so there is no expectXXX here.



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1465421196


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/inference/TypeInferenceOperandChecker.java:
##########
@@ -114,6 +117,24 @@ public boolean isOptional(int i) {
         return false;
     }
 
+    @Override
+    public List<RelDataType> paramTypes(RelDataTypeFactory typeFactory) {
+        throw new IllegalStateException("Should not be called");
+    }
+
+    @Override
+    public List<String> paramNames() {
+        return typeInference

Review Comment:
   The current limitation is that all functions or procedures with overloaded functions do not support named arguments, it is not because the function signature does not meet the requirements.  I think that it may not be possible to print the function signature here.



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "xuyangzhong (via GitHub)" <gi...@apache.org>.
xuyangzhong commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1462969824


##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -535,7 +535,43 @@ private static Stream<TestSpec> functionSpecs() {
                                         new String[] {}, new ArgumentTypeStrategy[] {}),
                                 TypeStrategies.explicit(
                                         DataTypes.ROW(DataTypes.FIELD("i", DataTypes.INT()))
-                                                .bridgedTo(RowData.class))));
+                                                .bridgedTo(RowData.class))),
+                TestSpec.forScalarFunction(
+                                "Scalar function with arguments hints",
+                                ArgumentHintScalarFunction.class)
+                        .expectNamedArguments("f1", "f2")
+                        .expectTypedArguments(DataTypes.STRING(), DataTypes.INT())
+                        .expectOutputMapping(
+                                InputTypeStrategies.sequence(
+                                        new String[] {"f1", "f2"},
+                                        new ArgumentTypeStrategy[] {
+                                            InputTypeStrategies.explicit(DataTypes.STRING()),
+                                            InputTypeStrategies.explicit(DataTypes.INT())
+                                        }),
+                                TypeStrategies.explicit(DataTypes.STRING())),
+                TestSpec.forScalarFunction(
+                                "Scalar function with arguments hints and inputs hints both defined",
+                                ArgumentsAndInputsScalarFunction.class)
+                        .expectErrorMessage(
+                                "Unable to support specifying both inputs and arguments at the same time"),
+                TestSpec.forScalarFunction(
+                                "Scalar function with ArgumentHint and DataTypeHint hints both defined",
+                                ArgumentsHintAndDataTypeHintScalarFunction.class)
+                        .expectErrorMessage(
+                                "ArgumentHint and DataTypeHint cannot be declared at the same time."),
+                TestSpec.forScalarFunction(
+                                "Scalar function with FunctionHint on class and method",
+                                FunctionHintOnClassAndMethod.class)
+                        .expectErrorMessage(
+                                "Unable to support specifying both inputs and arguments at the same time."),
+                TestSpec.forScalarFunction(
+                                "Scalar function with FunctionHint on class and method",
+                                MultiFunctionHintOnClass.class)
+                        .expectErrorMessage(
+                                "Considering all hints, the method should comply with the signature"),
+                TestSpec.forScalarFunction(

Review Comment:
   ditto



##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -535,7 +535,43 @@ private static Stream<TestSpec> functionSpecs() {
                                         new String[] {}, new ArgumentTypeStrategy[] {}),
                                 TypeStrategies.explicit(
                                         DataTypes.ROW(DataTypes.FIELD("i", DataTypes.INT()))
-                                                .bridgedTo(RowData.class))));
+                                                .bridgedTo(RowData.class))),
+                TestSpec.forScalarFunction(
+                                "Scalar function with arguments hints",
+                                ArgumentHintScalarFunction.class)
+                        .expectNamedArguments("f1", "f2")
+                        .expectTypedArguments(DataTypes.STRING(), DataTypes.INT())
+                        .expectOutputMapping(
+                                InputTypeStrategies.sequence(
+                                        new String[] {"f1", "f2"},
+                                        new ArgumentTypeStrategy[] {
+                                            InputTypeStrategies.explicit(DataTypes.STRING()),
+                                            InputTypeStrategies.explicit(DataTypes.INT())
+                                        }),
+                                TypeStrategies.explicit(DataTypes.STRING())),
+                TestSpec.forScalarFunction(
+                                "Scalar function with arguments hints and inputs hints both defined",
+                                ArgumentsAndInputsScalarFunction.class)
+                        .expectErrorMessage(
+                                "Unable to support specifying both inputs and arguments at the same time"),
+                TestSpec.forScalarFunction(
+                                "Scalar function with ArgumentHint and DataTypeHint hints both defined",
+                                ArgumentsHintAndDataTypeHintScalarFunction.class)
+                        .expectErrorMessage(
+                                "ArgumentHint and DataTypeHint cannot be declared at the same time."),
+                TestSpec.forScalarFunction(
+                                "Scalar function with FunctionHint on class and method",
+                                FunctionHintOnClassAndMethod.class)
+                        .expectErrorMessage(
+                                "Unable to support specifying both inputs and arguments at the same time."),
+                TestSpec.forScalarFunction(
+                                "Scalar function with FunctionHint on class and method",

Review Comment:
   nit: change the desc.



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/FunctionITCase.java:
##########
@@ -1056,7 +1057,111 @@ void testDynamicCatalogTableFunction() throws Exception {
     }
 
     @Test
-    void testInvalidUseOfScalarFunction() {
+    void testNamedArgumentsCatalogTableFunction() throws Exception {

Review Comment:
   nit: remove `Catalog` here



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/FunctionITCase.java:
##########
@@ -1056,7 +1057,111 @@ void testDynamicCatalogTableFunction() throws Exception {
     }
 
     @Test
-    void testInvalidUseOfScalarFunction() {
+    void testNamedArgumentsCatalogTableFunction() throws Exception {
+        final Row[] sinkData = new Row[] {Row.of("str1, str2")};
+
+        TestCollectionTableFactory.reset();
+
+        tEnv().executeSql("CREATE TABLE SinkTable(s STRING) WITH ('connector' = 'COLLECTION')");
+
+        tEnv().createFunction("NamedArgumentsTableFunction", NamedArgumentsTableFunction.class);
+        tEnv().executeSql(
+                        "INSERT INTO SinkTable "
+                                + "SELECT T1.s FROM TABLE(NamedArgumentsTableFunction(in2 => 'str2', in1 => 'str1')) AS T1(s) ")
+                .await();
+
+        assertThat(TestCollectionTableFactory.getResult()).containsExactlyInAnyOrder(sinkData);
+    }
+
+    @Test
+    void testNamedArgumentsScalarFunction() throws Exception {
+        final List<Row> sourceData =
+                Arrays.asList(Row.of(1, 2, "str1"), Row.of(3, 4, "str2"), Row.of(5, 6, "str3"));
+
+        final List<Row> sinkData =
+                Arrays.asList(Row.of(0, 0, "1: 2"), Row.of(0, 0, "3: 4"), Row.of(0, 0, "5: 6"));
+
+        TestCollectionTableFactory.reset();
+        TestCollectionTableFactory.initData(sourceData);
+
+        tEnv().executeSql(
+                        "CREATE TABLE TestTable(i1 INT NOT NULL, i2 INT NOT NULL, s1 STRING) WITH ('connector' = 'COLLECTION')");
+
+        tEnv().createTemporarySystemFunction(
+                        "NamedArgumentsScalarFunction", NamedArgumentsScalarFunction.class);
+        tEnv().executeSql(
+                        "INSERT INTO TestTable SELECT"
+                                + " 0 as i1, 0 as i2,"

Review Comment:
   nit: A little curious why i1 and i2 need to be set to 0



##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -700,7 +736,7 @@ private static Stream<TestSpec> procedureSpecs() {
                                 TypeStrategies.explicit(
                                         DataTypes.DOUBLE().notNull().bridgedTo(double.class))),
                 // named arguments with overloaded function
-                TestSpec.forProcedure(NamedArgumentsProcedure.class).expectNamedArguments("n"),
+                TestSpec.forProcedure(NamedArgumentsProcedure.class),

Review Comment:
   ditto



##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -427,8 +428,7 @@ private static Stream<TestSpec> functionSpecs() {
                                 "Could not find a publicly accessible method named 'eval'."),
 
                 // named arguments with overloaded function
-                TestSpec.forScalarFunction(NamedArgumentsScalarFunction.class)
-                        .expectNamedArguments("n"),
+                TestSpec.forScalarFunction(NamedArgumentsScalarFunction.class),

Review Comment:
   What about add `expectxxx` blocks here?



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "fsk119 (via GitHub)" <gi...@apache.org>.
fsk119 commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1464215746


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/annotation/ProcedureHint.java:
##########
@@ -139,6 +139,18 @@
      */
     String[] argumentNames() default {""};
 
+    /**
+     * Explicitly lists the argument that a procedure takes as input, including their names, types,
+     * and whether they are optional.
+     *
+     * <p>By default, it is recommended to use this parameter instead of {@link #input()}. If the
+     * type of argumentHint is not defined, it will be considered an invalid argument and an
+     * exception will be thrown. Additionally, both this parameter and {@link #input()} cannot be
+     * defined at the same time. If neither arguments nor {@link #input()} are defined,
+     * reflection-based extraction will be used.
+     */
+    ArgumentHint[] arguments() default {};

Review Comment:
   ditto



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/BaseMappingExtractor.java:
##########
@@ -362,7 +363,17 @@ static Optional<FunctionArgumentTemplate> tryExtractInputGroupArgument(
             Method method, int paramPos) {
         final Parameter parameter = method.getParameters()[paramPos];
         final DataTypeHint hint = parameter.getAnnotation(DataTypeHint.class);
-        if (hint != null) {
+        final ArgumentHint argumentHint = parameter.getAnnotation(ArgumentHint.class);
+        if (hint != null && argumentHint != null) {
+            throw extractionError(
+                    "ArgumentHint and DataTypeHint cannot be declared at the same time.");

Review Comment:
   nit: it's better to notify user which parameter is not correct.
   ```
    throw extractionError(
                       String.format(
                               "ArgumentHint and DataTypeHint cannot be declared at the same time for parameter '%s'.",
                               parameter.getName()));
   ```



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/inference/TypeInferenceOperandChecker.java:
##########
@@ -114,6 +117,24 @@ public boolean isOptional(int i) {
         return false;
     }
 
+    @Override
+    public List<RelDataType> paramTypes(RelDataTypeFactory typeFactory) {
+        throw new IllegalStateException("Should not be called");
+    }
+
+    @Override
+    public List<String> paramNames() {
+        return typeInference

Review Comment:
   Add more details exception. Print current required method method signature and the candidate method signatures.
   
   BTW, do you mean user can not use like this?
   ```
       public static class NamedArgumentsScalarFunction extends ScalarFunction {
           @FunctionHint(
                   output = @DataTypeHint("STRING"),
                   arguments = {
                       @ArgumentHint(name = "in1", type = @DataTypeHint("int")),
                       @ArgumentHint(name = "in2", type = @DataTypeHint("int"), isOptional = true)
                   })
           public String eval(Integer arg1, Integer arg2) {
               return (arg1 + ": " + arg2);
           }
   
           @FunctionHint(
                   output = @DataTypeHint("STRING"),
                   arguments = {
                           @ArgumentHint(name = "in1", type = @DataTypeHint("int")),
                           @ArgumentHint(name = "in2", type = @DataTypeHint("int"),  isOptional = true),
                           @ArgumentHint(name = "in3", type = @DataTypeHint("int"),  isOptional = true)
                   })
           public String eval(Integer arg1, Integer arg2, Integer arg3) {
               return (arg1 + ": " + arg2);
           }
       }
   ```
   
   ```
   SELECT udf(in1 => 1);
   ```
   



##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -1585,4 +1629,106 @@ public void eval(CompletableFuture<Long> f, int[] i) {}
     private static class DataTypeHintOnScalarFunctionAsync extends AsyncScalarFunction {
         public void eval(@DataTypeHint("ROW<i INT>") CompletableFuture<RowData> f) {}
     }
+
+    private static class ArgumentHintScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsAndInputsScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsHintAndDataTypeHintScalarFunction extends ScalarFunction {
+
+        public String eval(
+                @DataTypeHint("STRING") @ArgumentHint(name = "f1", type = @DataTypeHint("STRING"))
+                        String f1,
+                @ArgumentHint(name = "f2", type = @DataTypeHint("INTEGER")) int f2) {
+            return "";
+        }
+    }
+
+    @FunctionHint(
+            arguments = {
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    private static class InvalidFunctionHintOnClassAndMethod extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    @FunctionHint(
+            arguments = {
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    private static class ValidFunctionHintOnClassAndMethod extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    @FunctionHint(
+            arguments = {
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    @FunctionHint(
+            arguments = {
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    private static class ScalarFunctionWithFunctionHintConflictMethod extends ScalarFunction {
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsHintScalarFunctionWithOverloadedFunction extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f2")
+                })
+        public String eval(String f1, String f2) {

Review Comment:
   Can user specify like this
   ```
   public String eval(@ArgumentHint(type = @DataTypeHint("STRING"), name = "f1") String f1) {}
   ```



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/DataTypeTemplate.java:
##########
@@ -118,6 +119,10 @@ static DataTypeTemplate fromAnnotation(DataTypeFactory typeFactory, DataTypeHint
      * Creates an instance from the given {@link DataTypeHint} with a resolved data type if
      * available.
      */

Review Comment:
   The javadoc is not for this method.



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/inference/TypeInferenceOperandChecker.java:
##########
@@ -114,6 +117,24 @@ public boolean isOptional(int i) {
         return false;
     }
 
+    @Override
+    public List<RelDataType> paramTypes(RelDataTypeFactory typeFactory) {
+        throw new IllegalStateException("Should not be called");

Review Comment:
   Can you share some thoughts why this impossible? 



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/annotation/FunctionHint.java:
##########
@@ -148,6 +148,18 @@
      */
     String[] argumentNames() default {""};
 
+    /**
+     * Explicitly lists the argument that a function takes as input, including their names, types,
+     * and whether they are optional.
+     *
+     * <p>By default, it is recommended to use this parameter instead of {@link #input()}. If the
+     * type of argumentHint is not defined, it will be considered an invalid argument and an
+     * exception will be thrown. Additionally, both this parameter and {@link #input()} cannot be
+     * defined at the same time. If neither arguments nor {@link #input()} are defined,
+     * reflection-based extraction will be used.
+     */
+    ArgumentHint[] arguments() default {};

Review Comment:
   Please also add some examples at line 46 including how to specify an arugment is a variable-length argument.
   
   BTW, it seems that it uses `input` rather than `inputs` in the annotation. However, in the FLIP it uses `arguments` rather than `argument`.



##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3730,7 +3733,11 @@ private void checkRollUp(
                 // can be another SqlCall, or an SqlIdentifier.
                 checkRollUp(grandParent, parent, stripDot, scope, contextClause);
             } else {
-                List<? extends @Nullable SqlNode> children = ((SqlCall) stripDot).getOperandList();
+                // ----- FLINK MODIFICATION BEGIN -----
+                SqlCall call = (SqlCall) stripDot;
+                List<? extends @Nullable SqlNode> children =
+                        new SqlCallBinding(this, scope, call).operands();

Review Comment:
   Could you explain why modify this? If this is a bug, can you open a ticket on the calcite side, so we can track the progress.



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/FunctionTemplate.java:
##########
@@ -173,18 +183,45 @@ private static <T, H extends Annotation> T defaultAsNull(
 
     private static @Nullable FunctionSignatureTemplate createSignatureTemplate(
             DataTypeFactory typeFactory,
-            @Nullable DataTypeHint[] input,
+            @Nullable DataTypeHint[] inputs,
             @Nullable String[] argumentNames,
+            @Nullable ArgumentHint[] argumentHints,
             boolean isVarArg) {
-        if (input == null) {
+
+        String[] argumentHintNames;
+        DataTypeHint[] argumentHintTypes;
+
+        if (argumentHints != null && inputs != null) {
+            throw extractionError(
+                    "Unable to support specifying both inputs and arguments at the same time.");
+        }
+
+        if (argumentHints != null) {
+            argumentHintNames =
+                    Arrays.stream(argumentHints)
+                            .map(argumentHint -> defaultAsNull(argumentHint, ArgumentHint::name))
+                            .filter(Objects::nonNull)
+                            .toArray(String[]::new);
+            argumentHintTypes =
+                    Arrays.stream(argumentHints)
+                            .map(argumentHint -> defaultAsNull(argumentHint, ArgumentHint::type))
+                            .filter(Objects::nonNull)
+                            .toArray(DataTypeHint[]::new);
+        } else {
+            argumentHintTypes = inputs;
+            argumentHintNames = argumentNames;
+        }
+
+        if (argumentHintTypes == null) {

Review Comment:
   If user doesn't specify argument type, e.g. 
   ```
       public static class NamedArgumentsScalarFunction extends ScalarFunction {
           @FunctionHint(
                   output = @DataTypeHint("STRING"),
                   arguments = {@ArgumentHint(name = "in1"), @ArgumentHint(name = "in2")})
           public String eval(Integer arg1, Integer arg2) {
               return (arg1 + ": " + arg2);
           }
       }
   ```
   can we still use named argument in the SQL?



##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -1585,4 +1629,106 @@ public void eval(CompletableFuture<Long> f, int[] i) {}
     private static class DataTypeHintOnScalarFunctionAsync extends AsyncScalarFunction {
         public void eval(@DataTypeHint("ROW<i INT>") CompletableFuture<RowData> f) {}
     }
+
+    private static class ArgumentHintScalarFunction extends ScalarFunction {

Review Comment:
   if udf's argument hints have the same names, the planner throw exception to notify users?



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1465417066


##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3730,7 +3733,11 @@ private void checkRollUp(
                 // can be another SqlCall, or an SqlIdentifier.
                 checkRollUp(grandParent, parent, stripDot, scope, contextClause);
             } else {
-                List<? extends @Nullable SqlNode> children = ((SqlCall) stripDot).getOperandList();
+                // ----- FLINK MODIFICATION BEGIN -----
+                SqlCall call = (SqlCall) stripDot;
+                List<? extends @Nullable SqlNode> children =
+                        new SqlCallBinding(this, scope, call).operands();

Review Comment:
   I'll create a calcite jira later. 



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "fsk119 (via GitHub)" <gi...@apache.org>.
fsk119 merged PR #24106:
URL: https://github.com/apache/flink/pull/24106


-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "xuyangzhong (via GitHub)" <gi...@apache.org>.
xuyangzhong commented on PR #24106:
URL: https://github.com/apache/flink/pull/24106#issuecomment-1905675166

   cc @fsk119 for the double check.


-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1466053011


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/FunctionTemplate.java:
##########
@@ -173,18 +183,47 @@ private static <T, H extends Annotation> T defaultAsNull(
 
     private static @Nullable FunctionSignatureTemplate createSignatureTemplate(
             DataTypeFactory typeFactory,
-            @Nullable DataTypeHint[] input,
+            @Nullable DataTypeHint[] inputs,
             @Nullable String[] argumentNames,
+            @Nullable ArgumentHint[] argumentHints,
             boolean isVarArg) {
-        if (input == null) {
+
+        String[] argumentHintNames;
+        DataTypeHint[] argumentHintTypes;
+
+        if (argumentHints != null && inputs != null) {
+            throw extractionError(
+                    "Argument and input hints cannot be declared in the same function hint.");
+        }
+
+        if (argumentHints != null) {
+            argumentHintNames = new String[argumentHints.length];
+            argumentHintTypes = new DataTypeHint[argumentHints.length];
+            for (int i = 0; i < argumentHints.length; i++) {
+                ArgumentHint argumentHint = argumentHints[i];
+                argumentHintNames[i] = defaultAsNull(argumentHint, ArgumentHint::name);
+                argumentHintTypes[i] = defaultAsNull(argumentHint, ArgumentHint::type);
+                if (argumentHintNames[i] == null || argumentHintTypes[i] == null) {
+                    throw extractionError(

Review Comment:
   Updated, the current logic is that the argument name must be either all set or all not set.



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1465416608


##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -1585,4 +1629,106 @@ public void eval(CompletableFuture<Long> f, int[] i) {}
     private static class DataTypeHintOnScalarFunctionAsync extends AsyncScalarFunction {
         public void eval(@DataTypeHint("ROW<i INT>") CompletableFuture<RowData> f) {}
     }
+
+    private static class ArgumentHintScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsAndInputsScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsHintAndDataTypeHintScalarFunction extends ScalarFunction {
+
+        public String eval(
+                @DataTypeHint("STRING") @ArgumentHint(name = "f1", type = @DataTypeHint("STRING"))
+                        String f1,
+                @ArgumentHint(name = "f2", type = @DataTypeHint("INTEGER")) int f2) {
+            return "";
+        }
+    }
+
+    @FunctionHint(
+            arguments = {
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    private static class InvalidFunctionHintOnClassAndMethod extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    @FunctionHint(
+            arguments = {
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    private static class ValidFunctionHintOnClassAndMethod extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    @FunctionHint(
+            arguments = {
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    @FunctionHint(
+            arguments = {
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    private static class ScalarFunctionWithFunctionHintConflictMethod extends ScalarFunction {
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsHintScalarFunctionWithOverloadedFunction extends ScalarFunction {
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+
+        @FunctionHint(
+                arguments = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f2")
+                })
+        public String eval(String f1, String f2) {

Review Comment:
   Yes, I thinks we can supported this.  Updated. 



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1465416116


##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -1585,4 +1629,106 @@ public void eval(CompletableFuture<Long> f, int[] i) {}
     private static class DataTypeHintOnScalarFunctionAsync extends AsyncScalarFunction {
         public void eval(@DataTypeHint("ROW<i INT>") CompletableFuture<RowData> f) {}
     }
+
+    private static class ArgumentHintScalarFunction extends ScalarFunction {

Review Comment:
   I added a conflict check. 



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1466056607


##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -1585,4 +1659,143 @@ public void eval(CompletableFuture<Long> f, int[] i) {}
     private static class DataTypeHintOnScalarFunctionAsync extends AsyncScalarFunction {
         public void eval(@DataTypeHint("ROW<i INT>") CompletableFuture<RowData> f) {}
     }
+
+    private static class ArgumentHintScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintMissingTypeScalarFunction extends ScalarFunction {
+        @FunctionHint(argument = {@ArgumentHint(name = "f1"), @ArgumentHint(name = "f2")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintMissingNameScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING")),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"))
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintNameConflictScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(name = "in1", type = @DataTypeHint("STRING")),
+                    @ArgumentHint(name = "in1", type = @DataTypeHint("INTEGER"))
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintOnParameterScalarFunction extends ScalarFunction {
+        public String eval(
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "in1") String f1,
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "in2") int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsAndInputsScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsHintAndDataTypeHintScalarFunction extends ScalarFunction {
+
+        public String eval(
+                @DataTypeHint("STRING") @ArgumentHint(name = "f1", type = @DataTypeHint("STRING"))
+                        String f1,
+                @ArgumentHint(name = "f2", type = @DataTypeHint("INTEGER")) int f2) {
+            return "";
+        }
+    }
+
+    @FunctionHint(
+            argument = {
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    private static class InvalidFunctionHintOnClassAndMethod extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {

Review Comment:
   We can limit this when implementing optional arguments, currently there is no process to extract the optionals.



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1465412817


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/annotation/FunctionHint.java:
##########
@@ -148,6 +148,18 @@
      */
     String[] argumentNames() default {""};
 
+    /**
+     * Explicitly lists the argument that a function takes as input, including their names, types,
+     * and whether they are optional.
+     *
+     * <p>By default, it is recommended to use this parameter instead of {@link #input()}. If the
+     * type of argumentHint is not defined, it will be considered an invalid argument and an
+     * exception will be thrown. Additionally, both this parameter and {@link #input()} cannot be
+     * defined at the same time. If neither arguments nor {@link #input()} are defined,
+     * reflection-based extraction will be used.
+     */
+    ArgumentHint[] arguments() default {};

Review Comment:
   Changed to `argument`



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1466054255


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/inference/TypeInferenceOperandChecker.java:
##########
@@ -114,6 +119,40 @@ public boolean isOptional(int i) {
         return false;

Review Comment:
   Yes, supporting optional arguments will modify this part.



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "xuyangzhong (via GitHub)" <gi...@apache.org>.
xuyangzhong commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1461382300


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/annotation/ProcedureHint.java:
##########
@@ -139,6 +139,17 @@
      */
     String[] argumentNames() default {""};
 
+    /**
+     * Explicitly lists the argument that a procedure takes as input, including their names, types,
+     * and whether they are optional.
+     *
+     * <p>By default, argumentHint takes precedence over {@link #input()}. If the type of argument
+     * is not defined in this method, the type specified in {@link #input()} will be used instead.
+     * If {@link #input()} is also not defined, reflection-based extraction will be used and this
+     * parameter will be ignored.
+     */
+    ArgumentHint[] arguments() default @ArgumentHint;

Review Comment:
   ditto



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/annotation/FunctionHint.java:
##########
@@ -148,6 +148,17 @@
      */
     String[] argumentNames() default {""};
 
+    /**
+     * Explicitly lists the argument that a function takes as input, including their names, types,
+     * and whether they are optional.
+     *
+     * <p>By default, argumentHint takes precedence over {@link #input()}. If the type of argument
+     * is not defined in this method, the type specified in {@link #input()} will be used instead.
+     * If {@link #input()} is also not defined, reflection-based extraction will be used and this
+     * parameter will be ignored.
+     */
+    ArgumentHint[] arguments() default {@ArgumentHint};

Review Comment:
   Should we use `{}` as the default value?



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/annotation/ProcedureHint.java:
##########
@@ -139,6 +139,17 @@
      */
     String[] argumentNames() default {""};
 
+    /**
+     * Explicitly lists the argument that a procedure takes as input, including their names, types,
+     * and whether they are optional.
+     *
+     * <p>By default, argumentHint takes precedence over {@link #input()}. If the type of argument
+     * is not defined in this method, the type specified in {@link #input()} will be used instead.
+     * If {@link #input()} is also not defined, reflection-based extraction will be used and this
+     * parameter will be ignored.
+     */
+    ArgumentHint[] arguments() default @ArgumentHint;

Review Comment:
   Add changes to ProcedureHint back in the public interface in flip.



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/operations/SqlNodeToCallOperationTest.java:
##########
@@ -194,6 +206,24 @@ public int[] call(ProcedureContext context, int arg1, long arg2) {
         }
     }
 
+    private static class ProcedureWithNamedArguments implements Procedure {
+        @ProcedureHint(
+                input = {@DataTypeHint("INT"), @DataTypeHint("BIGINT")},
+                output = @DataTypeHint("INT"),
+                argumentNames = {"c", "d"})
+        public int[] call(ProcedureContext context, int arg1, long arg2) {

Review Comment:
   ditto, multi `call`.



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/ProcedureITCase.java:
##########
@@ -172,6 +175,24 @@ void testCallProcedure() {
                         Column.physical("age", DataTypes.INT().notNull().bridgedTo(int.class))));
     }
 
+    @Test
+    public void testNamedArguments() {
+        TableResult tableResult =
+                tEnv().executeSql("call `system`.named_args(d => 19, c => 'yuxia')");
+        verifyTableResult(
+                tableResult,
+                Collections.singletonList(Row.of("yuxia, 19")),
+                ResolvedSchema.of(Column.physical("result", DataTypes.STRING())));
+    }
+
+    @Test
+    public void testNamedArgumentsWithDefaultValue() {
+        // default value
+        Assertions.assertThatThrownBy(
+                        () -> tEnv().executeSql("call `system`.named_args(c => 'yuxia')"))
+                .isInstanceOf(ValidationException.class);

Review Comment:
   It's better to also validate the error message here.



##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/plan/nodes/logical/FlinkLogicalTableFunctionScan.scala:
##########
@@ -106,12 +106,15 @@ class FlinkLogicalTableFunctionScanConverter(config: Config) extends ConverterRu
     val scan = rel.asInstanceOf[LogicalTableFunctionScan]
     val traitSet = rel.getTraitSet.replace(FlinkConventions.LOGICAL).simplify()
     val newInputs = scan.getInputs.map(input => RelOptRule.convert(input, FlinkConventions.LOGICAL))
+    val rexCall = scan.getCall.asInstanceOf[RexCall];

Review Comment:
   It's better to add some comments here to let others know why we regenerate the rex call here.



##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -6056,7 +6056,8 @@ private void translateAgg(
             try {
                 // switch out of agg mode
                 bb.agg = null;
-                for (SqlNode operand : call.getOperandList()) {
+                for (SqlNode operand :

Review Comment:
   ditto



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/annotation/ArgumentHint.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.flink.table.annotation;
+
+import org.apache.flink.annotation.PublicEvolving;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * A hint that provides additional information about an argument.
+ *
+ * <p>An {@code ArgumentHint} can be used to provide hints about the name, optionality, and data
+ * type of argument.
+ *
+ * <p>It combines the functionality of {@link FunctionHint#argumentNames()} and {@link DataTypeHint}
+ * annotations to conveniently group argument-related information together in function declarations.
+ *
+ * <p>{@code @ArgumentHint(name = "in1", type = @DataTypeHint("STRING"), isOptional = false} an
+ * argument with the type String, named in1, and it is a required parameter.
+ */
+@PublicEvolving
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.TYPE, ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER})
+public @interface ArgumentHint {
+
+    /**
+     * The name of the argument.
+     *
+     * <p>This can be used to provide a descriptive name for the argument.
+     */
+    String name() default "";
+
+    /**
+     * Specifies whether the argument is optional or required.
+     *
+     * <p>If set to {@code true}, the argument is considered optional. If set to {@code false}, the

Review Comment:
   `If set to {@code true}, the argument is considered optional. And if the user does not specify this parameter when calling, 'null' will be passed in.`



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/annotation/ArgumentHint.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.flink.table.annotation;
+
+import org.apache.flink.annotation.PublicEvolving;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * A hint that provides additional information about an argument.
+ *
+ * <p>An {@code ArgumentHint} can be used to provide hints about the name, optionality, and data
+ * type of argument.
+ *
+ * <p>It combines the functionality of {@link FunctionHint#argumentNames()} and {@link DataTypeHint}
+ * annotations to conveniently group argument-related information together in function declarations.
+ *
+ * <p>{@code @ArgumentHint(name = "in1", type = @DataTypeHint("STRING"), isOptional = false} an

Review Comment:
   missing `is` before 'an'.
   `and it is a required parameter.` -> `cannot be omitted when calling.`



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/BaseMappingExtractor.java:
##########
@@ -362,11 +363,17 @@ static Optional<FunctionArgumentTemplate> tryExtractInputGroupArgument(
             Method method, int paramPos) {
         final Parameter parameter = method.getParameters()[paramPos];
         final DataTypeHint hint = parameter.getAnnotation(DataTypeHint.class);
+        final ArgumentHint argumentHint = parameter.getAnnotation(ArgumentHint.class);
         if (hint != null) {
             final DataTypeTemplate template = DataTypeTemplate.fromAnnotation(hint, null);
             if (template.inputGroup != null) {
                 return Optional.of(FunctionArgumentTemplate.of(template.inputGroup));
             }
+        } else if (argumentHint != null) {
+            final DataTypeTemplate template = DataTypeTemplate.fromAnnotation(argumentHint, null);

Review Comment:
   These changes don't appear to have been tested.
   And from the code, the priority of datatype hint (input) is greater than argument hint, right? This seems to be inconsistent with the comments on the interface



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/annotation/FunctionHint.java:
##########
@@ -148,6 +148,17 @@
      */
     String[] argumentNames() default {""};
 
+    /**
+     * Explicitly lists the argument that a function takes as input, including their names, types,
+     * and whether they are optional.
+     *
+     * <p>By default, argumentHint takes precedence over {@link #input()}. If the type of argument

Review Comment:
   Should we prohibit the use of arguments while the user is using input or argumentNames just like what discussed in the maillist: `We could deprecate 'input='. Or let both 'input' and 'arguments='
   coexist but never be defined at the same time.`



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/DataTypeTemplate.java:
##########
@@ -118,6 +119,22 @@ static DataTypeTemplate fromAnnotation(DataTypeFactory typeFactory, DataTypeHint
      * Creates an instance from the given {@link DataTypeHint} with a resolved data type if
      * available.
      */
+    static DataTypeTemplate fromAnnotation(ArgumentHint argumentHint, @Nullable DataType dataType) {
+        DataTypeHint hint = argumentHint.type();

Review Comment:
   `return fromAnnotation(argumentHint.type(), dataType);`



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/FunctionITCase.java:
##########
@@ -1509,6 +1618,25 @@ public void eval(Integer i) {
         }
     }
 
+    /** Function that returns a string or integer. */
+    public static class NamedArgumentsTableFunction extends TableFunction<Object> {
+        @FunctionHint(
+                input = {@DataTypeHint("STRING"), @DataTypeHint("STRING")},
+                output = @DataTypeHint("STRING"),
+                argumentNames = {"in1", "in2"})
+        public void eval(String arg1, String arg2) {
+            collect(arg1 + ", " + arg2);
+        }
+
+        @FunctionHint(
+                input = {@DataTypeHint("INT"), @DataTypeHint("INT")},
+                output = @DataTypeHint("STRING"),
+                argumentNames = {"in1", "in2"})
+        public void eval(Integer arg1, Integer arg2) {

Review Comment:
   From the flip, we should not support named argument for udx with multi `eval`, right?
   Ref ->  Limitation: `the UDX or procedure class  that support named parameters  can only have one eval method`



##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3730,7 +3730,9 @@ private void checkRollUp(
                 // can be another SqlCall, or an SqlIdentifier.
                 checkRollUp(grandParent, parent, stripDot, scope, contextClause);
             } else {
-                List<? extends @Nullable SqlNode> children = ((SqlCall) stripDot).getOperandList();
+                SqlCall call = (SqlCall) stripDot;

Review Comment:
   Add comments in this class, and add `// ----- FLINK MODIFICATION BEGIN -----` and `// ----- FLINK MODIFICATION END -----` here.



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlProcedureCallConverter.java:
##########
@@ -133,9 +133,17 @@ private List<RexNode> reduceOperands(SqlCall sqlCall, ConvertContext context) {
                         context.getSqlValidator().getTypeFactory());
         List<RexNode> rexNodes = new ArrayList<>();
         for (int i = 0; i < sqlCall.operandCount(); i++) {
-            RexNode rexNode = context.toRexNode(sqlCall.operand(i), inputRowType, null);
+            RexNode rexNode =

Review Comment:
   At first I wanted to understand why this change was needed. When I reverted this part of the change, I found that `ProcedureITCase#testNamedArguments` could run, but `SqlNodeToCallOperationTest#testWithNamedArguments` did not. When I dug deeper into the code, I found that another bug was in TypeInferenceOperandChecker#insertImplicitCasts (Line 158).
   `final LogicalType argumentType = toLogicalType(callBinding.getOperandType(i));` should be replaced into 
   `final LogicalType argumentType =
                       toLogicalType(
                               SqlTypeUtil.deriveType(callBinding, callBinding.operands().get(i)));`
   You can re-produce this bug by `ProcedureITCase#testNamedArguments`.
   Maybe we can unify the behavior of unwrapping more in advance.



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #24106:
URL: https://github.com/apache/flink/pull/24106#issuecomment-1894315428

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "a248bc76db79b94730b2107e05572f6874f7eb95",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "a248bc76db79b94730b2107e05572f6874f7eb95",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * a248bc76db79b94730b2107e05572f6874f7eb95 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on PR #24106:
URL: https://github.com/apache/flink/pull/24106#issuecomment-1905479204

   @xuyangzhong  Thank you for the review. I have already updated the code. Please take a look when you have time.


-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1465415619


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/FunctionTemplate.java:
##########
@@ -173,18 +183,45 @@ private static <T, H extends Annotation> T defaultAsNull(
 
     private static @Nullable FunctionSignatureTemplate createSignatureTemplate(
             DataTypeFactory typeFactory,
-            @Nullable DataTypeHint[] input,
+            @Nullable DataTypeHint[] inputs,
             @Nullable String[] argumentNames,
+            @Nullable ArgumentHint[] argumentHints,
             boolean isVarArg) {
-        if (input == null) {
+
+        String[] argumentHintNames;
+        DataTypeHint[] argumentHintTypes;
+
+        if (argumentHints != null && inputs != null) {
+            throw extractionError(
+                    "Unable to support specifying both inputs and arguments at the same time.");
+        }
+
+        if (argumentHints != null) {
+            argumentHintNames =
+                    Arrays.stream(argumentHints)
+                            .map(argumentHint -> defaultAsNull(argumentHint, ArgumentHint::name))
+                            .filter(Objects::nonNull)
+                            .toArray(String[]::new);
+            argumentHintTypes =
+                    Arrays.stream(argumentHints)
+                            .map(argumentHint -> defaultAsNull(argumentHint, ArgumentHint::type))
+                            .filter(Objects::nonNull)
+                            .toArray(DataTypeHint[]::new);
+        } else {
+            argumentHintTypes = inputs;
+            argumentHintNames = argumentNames;
+        }
+
+        if (argumentHintTypes == null) {

Review Comment:
   Currently, we require both "type" and "name" to be specified. Otherwise, it is not a valid ArgumentHint.



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1465413612


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/BaseMappingExtractor.java:
##########
@@ -362,7 +363,17 @@ static Optional<FunctionArgumentTemplate> tryExtractInputGroupArgument(
             Method method, int paramPos) {
         final Parameter parameter = method.getParameters()[paramPos];
         final DataTypeHint hint = parameter.getAnnotation(DataTypeHint.class);
-        if (hint != null) {
+        final ArgumentHint argumentHint = parameter.getAnnotation(ArgumentHint.class);
+        if (hint != null && argumentHint != null) {
+            throw extractionError(
+                    "ArgumentHint and DataTypeHint cannot be declared at the same time.");

Review Comment:
   parameters.getName can't get the real param name. I use position index instread. 



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "hackergin (via GitHub)" <gi...@apache.org>.
hackergin commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1465418253


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/inference/TypeInferenceOperandChecker.java:
##########
@@ -114,6 +117,24 @@ public boolean isOptional(int i) {
         return false;
     }
 
+    @Override
+    public List<RelDataType> paramTypes(RelDataTypeFactory typeFactory) {
+        throw new IllegalStateException("Should not be called");

Review Comment:
   The current usage does not involve this method, but the optional argument will need to be implemented. I have already made modifications to this part.



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "fsk119 (via GitHub)" <gi...@apache.org>.
fsk119 commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1465797504


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/FunctionTemplate.java:
##########
@@ -63,11 +64,13 @@ private FunctionTemplate(
      * types.
      */
     static FunctionTemplate fromAnnotation(DataTypeFactory typeFactory, FunctionHint hint) {
+

Review Comment:
   nit: revert this empty line



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/FunctionTemplate.java:
##########
@@ -78,11 +81,13 @@ static FunctionTemplate fromAnnotation(DataTypeFactory typeFactory, FunctionHint
      * data types.
      */
     static FunctionTemplate fromAnnotation(DataTypeFactory typeFactory, ProcedureHint hint) {
+

Review Comment:
   remove this empty line.



##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -1585,4 +1659,143 @@ public void eval(CompletableFuture<Long> f, int[] i) {}
     private static class DataTypeHintOnScalarFunctionAsync extends AsyncScalarFunction {
         public void eval(@DataTypeHint("ROW<i INT>") CompletableFuture<RowData> f) {}
     }
+
+    private static class ArgumentHintScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintMissingTypeScalarFunction extends ScalarFunction {
+        @FunctionHint(argument = {@ArgumentHint(name = "f1"), @ArgumentHint(name = "f2")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintMissingNameScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING")),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"))
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintNameConflictScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(name = "in1", type = @DataTypeHint("STRING")),
+                    @ArgumentHint(name = "in1", type = @DataTypeHint("INTEGER"))
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintOnParameterScalarFunction extends ScalarFunction {
+        public String eval(
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "in1") String f1,
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "in2") int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsAndInputsScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsHintAndDataTypeHintScalarFunction extends ScalarFunction {
+
+        public String eval(
+                @DataTypeHint("STRING") @ArgumentHint(name = "f1", type = @DataTypeHint("STRING"))
+                        String f1,
+                @ArgumentHint(name = "f2", type = @DataTypeHint("INTEGER")) int f2) {
+            return "";
+        }
+    }
+
+    @FunctionHint(
+            argument = {
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    private static class InvalidFunctionHintOnClassAndMethod extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {

Review Comment:
   BTW, if argument's DataType is not null, user can not specify optional = true.



##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -1585,4 +1659,143 @@ public void eval(CompletableFuture<Long> f, int[] i) {}
     private static class DataTypeHintOnScalarFunctionAsync extends AsyncScalarFunction {
         public void eval(@DataTypeHint("ROW<i INT>") CompletableFuture<RowData> f) {}
     }
+
+    private static class ArgumentHintScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")

Review Comment:
   argument `f2`'s type should INTEGER NOT NULL here.



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/FunctionTemplate.java:
##########
@@ -173,18 +183,47 @@ private static <T, H extends Annotation> T defaultAsNull(
 
     private static @Nullable FunctionSignatureTemplate createSignatureTemplate(
             DataTypeFactory typeFactory,
-            @Nullable DataTypeHint[] input,
+            @Nullable DataTypeHint[] inputs,
             @Nullable String[] argumentNames,
+            @Nullable ArgumentHint[] argumentHints,
             boolean isVarArg) {
-        if (input == null) {
+
+        String[] argumentHintNames;
+        DataTypeHint[] argumentHintTypes;
+
+        if (argumentHints != null && inputs != null) {
+            throw extractionError(
+                    "Argument and input hints cannot be declared in the same function hint.");
+        }
+
+        if (argumentHints != null) {
+            argumentHintNames = new String[argumentHints.length];
+            argumentHintTypes = new DataTypeHint[argumentHints.length];
+            for (int i = 0; i < argumentHints.length; i++) {
+                ArgumentHint argumentHint = argumentHints[i];
+                argumentHintNames[i] = defaultAsNull(argumentHint, ArgumentHint::name);
+                argumentHintTypes[i] = defaultAsNull(argumentHint, ArgumentHint::type);
+                if (argumentHintNames[i] == null || argumentHintTypes[i] == null) {
+                    throw extractionError(

Review Comment:
   I think arugment names can be null. For example, 
   ```
       public static class NamedArgumentsWithOverloadedScalarFunction extends ScalarFunction {
           @FunctionHint(
                   output = @DataTypeHint("STRING"),
                   argument = {
                       @ArgumentHint(type = @DataTypeHint("int")),
                       @ArgumentHint(type = @DataTypeHint("int"))
                   })
           public String eval(Integer arg1, Integer arg2) {
               return (arg1 + ": " + arg2);
           }
   }
   ```
   is same as 
   ```
       public static class NamedArgumentsWithOverloadedScalarFunction extends ScalarFunction {
           @FunctionHint(
                   output = @DataTypeHint("STRING"),
                   input = { @DataTypeHint ...})
           public String eval(Integer arg1, Integer arg2) {
               return (arg1 + ": " + arg2);
           }
   }
   ```
   



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/inference/TypeInferenceOperandChecker.java:
##########
@@ -114,6 +119,40 @@ public boolean isOptional(int i) {
         return false;

Review Comment:
   Do we need correct the behaviour?



-- 
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-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-34055][table] Introduce a new annotation for named parameters [flink]

Posted by "fsk119 (via GitHub)" <gi...@apache.org>.
fsk119 commented on code in PR #24106:
URL: https://github.com/apache/flink/pull/24106#discussion_r1466079035


##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/extraction/TypeInferenceExtractorTest.java:
##########
@@ -1585,4 +1659,143 @@ public void eval(CompletableFuture<Long> f, int[] i) {}
     private static class DataTypeHintOnScalarFunctionAsync extends AsyncScalarFunction {
         public void eval(@DataTypeHint("ROW<i INT>") CompletableFuture<RowData> f) {}
     }
+
+    private static class ArgumentHintScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintMissingTypeScalarFunction extends ScalarFunction {
+        @FunctionHint(argument = {@ArgumentHint(name = "f1"), @ArgumentHint(name = "f2")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintMissingNameScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING")),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"))
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintNameConflictScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(name = "in1", type = @DataTypeHint("STRING")),
+                    @ArgumentHint(name = "in1", type = @DataTypeHint("INTEGER"))
+                })
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentHintOnParameterScalarFunction extends ScalarFunction {
+        public String eval(
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "in1") String f1,
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "in2") int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsAndInputsScalarFunction extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {
+            return "";
+        }
+    }
+
+    private static class ArgumentsHintAndDataTypeHintScalarFunction extends ScalarFunction {
+
+        public String eval(
+                @DataTypeHint("STRING") @ArgumentHint(name = "f1", type = @DataTypeHint("STRING"))
+                        String f1,
+                @ArgumentHint(name = "f2", type = @DataTypeHint("INTEGER")) int f2) {
+            return "";
+        }
+    }
+
+    @FunctionHint(
+            argument = {
+                @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+            })
+    private static class InvalidFunctionHintOnClassAndMethod extends ScalarFunction {
+        @FunctionHint(
+                argument = {
+                    @ArgumentHint(type = @DataTypeHint("STRING"), name = "f1"),
+                    @ArgumentHint(type = @DataTypeHint("INTEGER"), name = "f2")
+                },
+                input = {@DataTypeHint("STRING"), @DataTypeHint("INTEGER")})
+        public String eval(String f1, int f2) {

Review Comment:
   @hackergin can you open a ticket as a future work. 



-- 
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-unsubscribe@flink.apache.org

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