Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flink] add table comment for create table in sync action #1877

Closed

Conversation

zhangjun0x01
Copy link
Contributor

Purpose

in flink sync action , sync the table comment when create new table.

Tests

API and Format

Documentation

@@ -79,13 +79,13 @@ static Connection getConnection(Configuration mySqlConfig, boolean tinyint1NotBo
throws Exception {
String url =
String.format(
"jdbc:mysql://%s:%d%s",
"jdbc:mysql://%s:%d?useInformationSchema=true%s",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use an option to open this feature?
I don't know how much impact it has in performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the param in default true in mysql 8.0.3 (doc) ,but it is default false in mysql 5.7, if we add an option and when we set it to false ,do we should set it to false explicitly in mysql8 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Default auto?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or default we can just disable table comments.

@zhangjun0x01 zhangjun0x01 force-pushed the add_table_comment_for_sync branch 2 times, most recently from e0c02d7 to 7dea8cd Compare August 29, 2023 01:20
@zhangjun0x01 zhangjun0x01 force-pushed the add_table_comment_for_sync branch from 7dea8cd to a39151d Compare September 1, 2023 13:48
@zhuangchong
Copy link
Contributor

Already supported, I will close this PR first. If you have any questions, you can open it again.

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.

3 participants