openinx commented on a change in pull request #3061:
URL: https://github.com/apache/iceberg/pull/3061#discussion_r701538454
##########
File path: flink/src/main/java/org/apache/iceberg/flink/sink/FlinkSink.java
##########
@@ -249,7 +249,7 @@ public Builder uidPrefix(String newPrefix) {
return this;
}
- public DataStreamSink<RowData> build() {
+ public DataStreamSink<Void> build() {
Review comment:
@stevenzwu , Yes, it's a breaking changes for flink DataStream API, it's
necessary to maintain the compatibility for at least one major iceberg release.
@rdblue , for flink DataStream operators chain, the returned
DataStreamSink should has the same return type as the last chained operator,
so it does not make sense to return a DataStreamSink with an arbitrary type as
user want.
For @stevenzwu 's second suggestion, I think it's still necessary to
return a `DataStreamSink<Void>` in case of people want to set their own `name`,
`uid`, `ChainingStrategy` etc, though less people will really do that but we
need to.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
