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 create comment to table spec #7416

Closed
Allam76 opened this issue Jan 26, 2024 · 4 comments
Closed

add table create comment to table spec #7416

Allam76 opened this issue Jan 26, 2024 · 4 comments
Assignees
Labels
customer issue enhancement New feature or request sql Issue with SQL

Comments

@Allam76
Copy link

Allam76 commented Jan 26, 2024

mysql allows comments on create table statement, both on the table clause itself as well as on the columns. The column spec has this implemented but for the table this is missing.

The parsed AST does contain the comment so it is merely about passing it to the tablespec.
Would it be possible to ask to add this?

As in

CREATE TABLE test(id text) COMMENT="This is a comment about table";

I would assume it should be added here:

https://github.com/dolthub/go-mysql-server/blob/e42adc9dbc2869bab77e0012c6f01f0d96dee0ab/sql/planbuilder/ddl.go#L244-L250

Many thanks
Martin

@fulghum
Copy link
Contributor

fulghum commented Jan 26, 2024

Hi @Allam76, thanks for contacting us. It makes sense for us to support those comments at the table level, too. Looks like we just need to pull the comment out of the AST, plug it into TableSpec, update the schema serialization to store/load that new field, and also add that into the merge logic so that table comments are merged properly.

Thanks for requesting this one! We'll start looking into the implementation details.

I'm going to slide this issue over to the dolt repo, since there's some additional Dolt specific work to support this (i.e. table schema merging) and for better visibility.

@fulghum fulghum transferred this issue from dolthub/go-mysql-server Jan 26, 2024
@timsehn timsehn added enhancement New feature or request sql Issue with SQL labels Jan 26, 2024
@fulghum fulghum self-assigned this Jan 30, 2024
@fulghum
Copy link
Contributor

fulghum commented Jan 31, 2024

I've got create table statements that contain table comments working in the GMS layer and in the Dolt layer in my local workspace – when you run show create table the table comments are still present. I need to tidy up the implementation a bit and then get it out for review today.

Note that I ran into a couple of snags along the way, so full support for table level comments will require a bit more follow up work, but we can iteratively release functionality. The most notable limitation is that this first iteration can't yet support comments that contain the single quote character. This is due to a limitation in how we are parsing this content in our SQL parser, and how it is assembled back into a string that needs to be reparsed at the GMS layer. Unfortunately, that process loses some information and we can't disambiguate single quotes. The fix is pretty straightforward – we need to change the SQL parser to parse table options into structures, instead of back into a string. It'll just take some time to untangle that, and I think it's best done as a follow up.

@bpf120
Copy link

bpf120 commented Jan 31, 2024

@Allam76 , thanks for filing and using Dolt! We'd love to learn more about your use case. Feel free to email me or swing by our Discord if you want to share.

@fulghum
Copy link
Contributor

fulghum commented Feb 1, 2024

@Allam76, support for persisting table comments has been merged into go-mysql-server, as well as dolt. If you pick up the latest commit from main for go-mysql-server, you'll see the new support. dolthub/go-mysql-server#2307 explains a few limitations on this first iteration of our support for table level comments. Feel free to let us know if any of those cause problems and we see if we can prioritize more work here.

I'll go ahead and resolve this issue now that the support is merged in, but let us know if you need anything else!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer issue enhancement New feature or request sql Issue with SQL
Projects
None yet
Development

No branches or pull requests

5 participants