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

Allow primary key comment on create table statement #36385

Conversation

guigs
Copy link
Contributor

@guigs guigs commented Jun 3, 2019

This PR adds support for primary key column comment with primary_key_comment option to create_table.

Before this schema dump/load was not able to keep a comment that was added to a primary key column.

This PR complements #36384.

@guigs guigs force-pushed the allow-primary-key-comment-on-create-table-statement branch from 185e476 to 9cbabae Compare June 3, 2019 11:43
@kamipo
Copy link
Member

kamipo commented Jun 3, 2019

I'm not prefer to add new column option for primary key on create_table.
If we already be able to create a pk column with comment (e.g. t.primary_key :id, comment: "Primary key comment"), it could be implemented to dump the schema without new option.

@guigs
Copy link
Contributor Author

guigs commented Jun 3, 2019

Yea, I was not very happy to add a new option either. Do you suggest that whenever the pk column has a comment, we should add explicit primary key statement t.primary_key in schema? I can investigate that possibility.

… option to `create_table`

Before this schema dump/load was not able to keep a comment that was added to a primary key column.
@guigs guigs force-pushed the allow-primary-key-comment-on-create-table-statement branch from 9cbabae to f634630 Compare June 3, 2019 13:22
@rails-bot
Copy link

rails-bot bot commented Dec 17, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 17, 2019
@rails-bot rails-bot bot closed this Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants