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

Skip Alter ADD CONSTRAINT when changing the table schema #39

Merged
merged 4 commits into from
Oct 12, 2022

Conversation

youpy
Copy link
Contributor

@youpy youpy commented Oct 11, 2022

related: go-rel/sql#39

@youpy youpy changed the title skip unsupported table definitions migration: skip ADD CONSTRAINT when changing the table schema Oct 11, 2022
sqlite3.go Outdated
_, ok := def.(rel.Key)
// https://www.sqlite.org/omitted.html
// > Only the RENAME TABLE, ADD COLUMN, RENAME COLUMN, and DROP COLUMN variants of the ALTER TABLE command are supported.
if ok && table.Op == rel.SchemaAlter {
Copy link
Member

Choose a reason for hiding this comment

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

won't this skip RENAME TABLE, ADD COLUMN, RENAME COLUMN, and DROP COLUMN as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. I added tests to verify that they are not skipped.

Copy link
Member

Choose a reason for hiding this comment

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

oh, I missed _, ok := def.(rel.Key)

can you move above statement inside if table.Op == rel.SchemaAlter? so there's no need to try casting when Op is not SchemaAlter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Copy link
Member

@Fs02 Fs02 left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you

@Fs02 Fs02 changed the title migration: skip ADD CONSTRAINT when changing the table schema Skip Alter ADD CONSTRAINT when changing the table schema Oct 12, 2022
@Fs02 Fs02 merged commit d45378c into go-rel:main Oct 12, 2022
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.

None yet

2 participants