wuchong commented on a change in pull request #9994: [FLINK-14322][table-api]
Add watermark information in TableSchema
URL: https://github.com/apache/flink/pull/9994#discussion_r341473592
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/api/TableSchema.java
##########
@@ -343,13 +396,30 @@ public Builder field(String name, TypeInformation<?>
typeInfo) {
return field(name, fromLegacyInfoToDataType(typeInfo));
}
+ /**
+ * Specifies the previously defined field as an event-time
attribute and specifies the watermark strategy.
+ *
+ * @param rowtimeAttribute the field name as a rowtime
attribute, can be a nested field using dot separator.
+ * @param watermarkStrategy the watermark strategy expression
+ */
+ public Builder watermark(String rowtimeAttribute,
ResolvedExpression watermarkStrategy) {
Review comment:
As we discussed in FLIP-80, we will use the serialize (resolved) expression
to string, this happens when serialize CatalogTable to properties. And
deserialize string to unresolved expression, this happens when create
`CatalogTable` from properties in `Catalog`. However, we missed that converting
unresovled expression to resolved expression requires `ExpressionResolver`
which is not held by `Catalog`. If the TableSchema builder requires
`ResolvedExpression`, it's impossible to create a `TableSchema` or
`CatalogTable` for `Catalog`.
Let me sum up, I agree with @KurtYoung that using `ResolveExpression` here
may can't work in the future, so I would like change it to :
```java
public Builder watermark(String rowtimeAttribute, String
watermarkExpression, DataType watermarkExprOutputType) {
```
The `ResolvedExpression getWatermarkStrategy()` in `WatermarkSpec` will be
removed, and will expose `String getWatermarkExpressionString()` and `DataType
getWatermarkExprOutputType()`.
What do you think?
----------------------------------------------------------------
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
