dawidwys commented on code in PR #23634:
URL: https://github.com/apache/flink/pull/23634#discussion_r1388150505


##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java:
##########
@@ -636,7 +639,45 @@ ANY, explicit(DataTypes.INT())
                         .expectSignature("f(<ARRAY>, <ARRAY ELEMENT>)")
                         .expectArgumentTypes(
                                 
DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(),
-                                DataTypes.INT()));
+                                DataTypes.INT()),
+                TestSpec.forStrategy(
+                                "PROCTIME type strategy",
+                                
SpecificInputTypeStrategies.windowTimeIndicator(
+                                        TimestampKind.PROCTIME))
+                        
.calledWithArgumentTypes(timeIndicatorType(TimestampKind.PROCTIME))
+                        .expectSignature("f(<WINDOW REFERENCE>)")
+                        
.expectArgumentTypes(timeIndicatorType(TimestampKind.PROCTIME)),
+                TestSpec.forStrategy(
+                                "PROCTIME type strategy on non time indicator",
+                                
SpecificInputTypeStrategies.windowTimeIndicator(
+                                        TimestampKind.PROCTIME))
+                        .calledWithArgumentTypes(DataTypes.BIGINT())
+                        .expectErrorMessage("Reference to a rowtime or 
proctime window required."),
+                TestSpec.forStrategy(
+                                "ROWTIME type strategy",
+                                
SpecificInputTypeStrategies.windowTimeIndicator(
+                                        TimestampKind.ROWTIME))
+                        
.calledWithArgumentTypes(timeIndicatorType(TimestampKind.ROWTIME))
+                        .expectSignature("f(<WINDOW REFERENCE>)")
+                        
.expectArgumentTypes(timeIndicatorType(TimestampKind.ROWTIME)),
+                TestSpec.forStrategy(
+                                "ROWTIME type strategy on proctime indicator",
+                                
SpecificInputTypeStrategies.windowTimeIndicator(
+                                        TimestampKind.ROWTIME))
+                        
.calledWithArgumentTypes(timeIndicatorType(TimestampKind.PROCTIME))
+                        .expectErrorMessage(
+                                "A proctime window cannot provide a rowtime 
attribute."),

Review Comment:
   Yeah, that's what I meant by "timeindicators are tricky".
   
   The thing is the validation in the API layer is not perfect. It has already 
lots of false positives at different locations. By false positives I mean we 
can not fully trust if a column is or is not a proctime. That's some general 
background.
   
   Now to the ROWTIME/PROCTIME. Rowtime strictly requires Watermarks to be 
working. Watermarks are only present  if you do a rowtime window. PROCTIME on 
the other hand tells the time when a record was processed. This theoretically 
can always be returned. It could be returned both for rowtime and proctime 
windows.
   
   I can add a test for that.



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

Reply via email to