[GitHub] [flink] godfreyhe commented on a change in pull request #14200: [FLINK-20306][table-planner-blink] Throw a meaningful exception when accessing temporal table as of constant timestamp

[GitBox]

godfreyhe commented on a change in pull request #14200:
URL: https://github.com/apache/flink/pull/14200#discussion_r530088484



##########
File path: 
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/rules/common/CommonTemporalTableJoinRule.scala
##########
@@ -44,20 +44,15 @@ trait CommonTemporalTableJoinRule {
 
     // period specification check
     snapshot.getPeriod match {
-      // it's left table's field, pass
-      case r: RexFieldAccess if 
r.getReferenceExpr.isInstanceOf[RexCorrelVariable] =>
+      // it should be left table's field and is a time attribute
+      case r: RexFieldAccess
+        if r.getType.isInstanceOf[TimeIndicatorRelDataType] &&
+          r.getReferenceExpr.isInstanceOf[RexCorrelVariable] => // pass
       case _ =>
         throw new TableException("Temporal table join currently only supports 
" +
-          "'FOR SYSTEM_TIME AS OF' left table's time attribute field, doesn't 
support 'PROCTIME()'")
+          "'FOR SYSTEM_TIME AS OF' left table's time attribute field.")
     }
 
-    snapshot.getPeriod.getType match {
-      // supports both event-time and processing time
-      case t: TimeIndicatorRelDataType =>
-      case _ =>
-        throw new TableException("Temporal table join currently only supports 
" +
-          "'FOR SYSTEM_TIME AS OF' left table's time attribute field")
-    }

Review comment:
       Do we check the period type in other place ?




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