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

Add table comments interface in base JDBC and implement in MySQL #11211

Merged
merged 4 commits into from
Mar 18, 2022

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Feb 28, 2022

Description

Add table comments interface in base JDBC and implement in MySQL

Fixes #2517

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# MySQL
* Add support for table comments. ({issue}`11211`)

@cla-bot cla-bot bot added the cla-signed label Feb 28, 2022
@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from 43db833 to b1ae7c4 Compare February 28, 2022 08:57
@hashhar hashhar self-requested a review March 1, 2022 12:47
@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from b1ae7c4 to 30751a1 Compare March 1, 2022 13:32
@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from 30751a1 to f57fdef Compare March 1, 2022 23:25
@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch 6 times, most recently from 156170c to f5dac2c Compare March 7, 2022 23:53
@ebyhr ebyhr requested a review from findepi March 8, 2022 09:42
@findepi findepi removed their request for review March 8, 2022 16:01
@ebyhr ebyhr requested review from hashhar and removed request for hashhar March 12, 2022 05:33
@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from f5dac2c to c95e0f4 Compare March 15, 2022 05:57
Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

Should we also test that creating a table without comments indeed doesn't return any comments in the info schema and SHOW CREATE output?

Existing test in BaseConnectorTest#testCommentTable only tests setting comments and creating table with comments. Note that testShowCreateTable doesn't test this since it checks against TPCH tables which may be created without using Trino during test setup.

LGTM % this comment

@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from c95e0f4 to c116960 Compare March 15, 2022 07:30
@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from c116960 to e0f3681 Compare March 16, 2022 00:52
@ebyhr
Copy link
Member Author

ebyhr commented Mar 16, 2022

@hashhar I modified TestingH2JdbcClient and BaseConnectorTest.getTableComment() after your last review. Please take another look.

@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from e0f3681 to 85a7f51 Compare March 16, 2022 10:20
@ebyhr
Copy link
Member Author

ebyhr commented Mar 16, 2022

Rebased on upstream to resolve conflicts.

@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from 85a7f51 to 4a71c31 Compare March 18, 2022 01:02
@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from 4a71c31 to 4455c03 Compare March 18, 2022 01:03
@ebyhr ebyhr force-pushed the ebi/jdbc-comment-table branch from 4455c03 to 3e06544 Compare March 18, 2022 01:04
@ebyhr ebyhr merged commit 942b448 into trinodb:master Mar 18, 2022
@ebyhr ebyhr deleted the ebi/jdbc-comment-table branch March 18, 2022 03:20
@ebyhr ebyhr mentioned this pull request Mar 18, 2022
@github-actions github-actions bot added this to the 375 milestone Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Expose table comment in JDBC based connectors
3 participants