twalthr commented on a change in pull request #8655: [FLINK-12254][table]
Update field references to new type system
URL: https://github.com/apache/flink/pull/8655#discussion_r291508131
##########
File path:
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/operations/AggregateOperationFactory.java
##########
@@ -257,28 +260,31 @@ private FieldReferenceExpression
getValidatedTimeAttribute(GroupWindow window, E
}
FieldReferenceExpression timeField = (FieldReferenceExpression)
timeFieldExpr;
- validateTimeAttributeType(timeField);
+
+ final LogicalType timeFieldType =
timeField.getOutputDataType().getLogicalType();
+
+ validateTimeAttributeType(timeFieldType);
+
return timeField;
}
- private void validateTimeAttributeType(FieldReferenceExpression
timeField) {
- TypeInformation<?> timeFieldType = timeField.getResultType();
+ private void validateTimeAttributeType(LogicalType timeFieldType) {
if (isStreaming) {
validateStreamTimeAttribute(timeFieldType);
} else {
validateBatchTimeAttribute(timeFieldType);
}
}
- private void validateBatchTimeAttribute(TypeInformation<?>
timeFieldType) {
- if (!(timeFieldType instanceof SqlTimeTypeInfo || timeFieldType
== LONG_TYPE_INFO)) {
+ private void validateBatchTimeAttribute(LogicalType timeFieldType) {
+ if (!(hasRoot(timeFieldType, TIMESTAMP_WITHOUT_TIME_ZONE) ||
hasRoot(timeFieldType, BIGINT))) {
Review comment:
I think the planners need to verify the types that are entering at the
beginning. The validation in the API should be performed logically otherwise we
end up in an validation hell.
If something goes wrong a users has two fallbacks, either use the old type
system or fully specify the DataType such that a legacy type info converter
understands it.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
