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_r365764205
##########
File path:
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/rules/logical/BatchLogicalWindowAggregateRule.scala
##########
@@ -62,27 +61,14 @@ class BatchLogicalWindowAggregateRule
throw new ValidationException("Window can not be defined over "
+ "a proctime attribute column for batch mode")
}
- operand match {
- case c: RexCall if c.getKind == SqlKind.CAST =>
- getTimeFieldReference(c.getOperands.get(0), windowExprIdx, rowType)
- // match TUMBLE_ROWTIME and TUMBLE_PROCTIME
- case c: RexCall if c.getOperands.size() == 1 &&
- FlinkTypeFactory.isTimeIndicatorType(c.getType) =>
- new FieldReferenceExpression(
- rowType.getFieldList.get(windowExprIdx).getName,
- fromLogicalTypeToDataType(toLogicalType(c.getType)),
- 0, // only one input, should always be 0
- windowExprIdx)
- case ref: RexInputRef =>
- // resolve field name of window attribute
- val fieldName = rowType.getFieldList.get(ref.getIndex).getName
- val fieldType = rowType.getFieldList.get(ref.getIndex).getType
- new FieldReferenceExpression(
- fieldName,
- fromLogicalTypeToDataType(toLogicalType(fieldType)),
- 0, // only one input, should always be 0
- windowExprIdx)
- }
+
+ val fieldName = rowType.getFieldList.get(windowExprIdx).getName
+ val fieldType = rowType.getFieldList.get(windowExprIdx).getType
+ new FieldReferenceExpression(
+ fieldName,
+ fromLogicalTypeToDataType(toLogicalType(fieldType)),
+ 0,
+ windowExprIdx)
Review comment:
The fix looks good to me. However, could you add some comments or change the
parameter `windowExprIdx` to `timeAttributeIndex` ? As we know, `windowExprIdx`
is `timeAttributeIndex` and so the rowtime field reference is accessed by
`windowExprIdx`.
The same to `StreamLogicalWindowAggregateRule`.
----------------------------------------------------------------
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
