dawidwys commented on a change in pull request #11902:
URL: https://github.com/apache/flink/pull/11902#discussion_r415587181



##########
File path: 
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/extraction/utils/ExtractionUtils.java
##########
@@ -415,13 +418,17 @@ public static boolean hasStructuredFieldGetter(Class<?> 
clazz, Field field) {
        public static <T extends Annotation> Set<T> collectAnnotationsOfClass(

Review comment:
       Can we go over the methods in this class and reduce the scope of them? I 
always feel uncomfortable returning a sorted Set with an implicit contract.

##########
File path: 
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java
##########
@@ -154,19 +154,41 @@
                                
.expectSignature("f(STRING)\nf(INT)\nf(BOOLEAN)")
                                
.expectArgumentTypes(DataTypes.INT().bridgedTo(int.class)),
 
-                       // OR with implicit casting
                        TestSpec
                                .forStrategy(
+                                       "OR with implicit casting",
                                        or(
                                                
explicitSequence(DataTypes.TINYINT()),
                                                
explicitSequence(DataTypes.INT()),
                                                
explicitSequence(DataTypes.BIGINT())))
                                .calledWithArgumentTypes(DataTypes.SMALLINT())
                                .expectArgumentTypes(DataTypes.INT()),
 
-                       // invalid type in OR
                        TestSpec
-                               
.forStrategy(or(explicitSequence(DataTypes.INT()), 
explicitSequence(DataTypes.STRING())))
+                               .forStrategy(

Review comment:
       Shall we add a test that check the first input strategy is chosen if 
multiple match with an implicit cast?




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

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


Reply via email to