Skip to content

Conversation

@polyzos
Copy link
Contributor

@polyzos polyzos commented Apr 4, 2025

This PR makes the Flink Sink generic so it can later be used by the FlussSink for the datastream API and handle various data types. It also introduces a ConvertingSinkWriter for converting between various types and RowData.

@polyzos polyzos requested a review from wuchong April 4, 2025 06:57
Copy link
Member

@wuchong wuchong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @polyzos. I’m wondering why we aren’t introducing a FlussSerializationSchema that directly converts T into InternalRow. This approach would be consistent with how we handled the generic source, where we directly worked with InternalRow.

In this PR, we introduced the ConvertingSinkWriter, which first converts T into RowData and then subsequently converts RowData into InternalRow. However, the intermediate conversion to RowData seems unnecessary since our ultimate goal is to work with InternalRow directly.

Wouldn’t it make more sense to bypass RowData entirely and streamline the process by converting T directly into InternalRow?

@polyzos
Copy link
Contributor Author

polyzos commented Apr 27, 2025

@wuchong ConvertingSinkWriter was introduced for convenience, as I was getting some compilation issues with the current implementation. I figured out indeed a better approach though... since this PR is mainly around introducing a ConvertingSinkWriter to achieve make the class generic, I will close this and open a new PR from latest branch to avoid deletion etc.

@polyzos polyzos closed this Apr 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants