wuchong commented on a change in pull request #10784: [FLINK-15494] Fix time 
field index wrong in LogicalWindowAggregateRul…
URL: https://github.com/apache/flink/pull/10784#discussion_r365765439
 
 

 ##########
 File path: 
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/rules/logical/LogicalWindowAggregateRuleBase.scala
 ##########
 @@ -88,6 +85,9 @@ abstract class LogicalWindowAggregateRuleBase(description: 
String)
       .project(project.getChildExps.updated(windowExprIdx, 
inAggGroupExpression))
       .build()
 
+    // translate window against newProject.
+    val window = translateWindow(windowExpr, windowExprIdx, 
newProject.getRowType)
 
 Review comment:
   Because `translateWindow` is postponed after 
`getInAggregateGroupExpression`, so if the time field is not a time attribute, 
the exception will be thrown from `getInAggregateGroupExpression`. But the 
exception there is not clear enought. Could you improve the exception in 
`getInAggregateGroupExpression`? For example: 
   
   ```scala
   s"Window aggregate can only be defined over a time attribute column, but 
${timeAttribute.getType} encountered."
   ```

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


With regards,
Apache Git Services

Reply via email to