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