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

Check that column arity is preserved by AlterEnum on MySQL #1032

Closed
tomhoule opened this issue Aug 15, 2020 · 1 comment · Fixed by #1058
Closed

Check that column arity is preserved by AlterEnum on MySQL #1032

tomhoule opened this issue Aug 15, 2020 · 1 comment · Fixed by #1058
Assignees
Labels
tech/engines/migration engine Issue in the Migration Engine tech/engines Issue for tech Engines.
Milestone

Comments

@tomhoule
Copy link
Contributor

No description provided.

@tomhoule tomhoule self-assigned this Aug 15, 2020
@albertoperdomo albertoperdomo added this to the Backlog 2.6.0 milestone Aug 19, 2020
tomhoule added a commit that referenced this issue Aug 21, 2020
Problem: AlterEnum steps on MySQL would only attempt to change the type of the column, but lost the arity in the process.

Two solutions were envisaged:

Change AlterEnum rendering to share most of the architecture of ColumnChange::AlterColumn, so it would produce a valid column migration.
Since enum changes are implemented by a MODIFY on the column, absorb enum diffing into column diffing on MySQL.
This PR went with (2), because (1) has the big downside of producing two steps for a change that can be done in a single statement, and it sticks closer to the way enums work on MySQL — that way it will not break as easily if we change how we represent MySQL enums in sql-schema-describer.

closes #1032
@tomhoule
Copy link
Contributor Author

It isn't. See #1058 for the fix.

tomhoule added a commit that referenced this issue Aug 21, 2020
Problem: AlterEnum steps on MySQL would only attempt to change the type of the column, but lost the arity in the process.

Two solutions were envisaged:

Change AlterEnum rendering to share most of the architecture of ColumnChange::AlterColumn, so it would produce a valid column migration.
Since enum changes are implemented by a MODIFY on the column, absorb enum diffing into column diffing on MySQL.
This PR went with (2), because (1) has the big downside of producing two steps for a change that can be done in a single statement, and it sticks closer to the way enums work on MySQL — that way it will not break as easily if we change how we represent MySQL enums in sql-schema-describer.

closes #1032
@tomhoule tomhoule modified the milestones: Backlog 2.6.0, Release 2.6.0 Aug 21, 2020
@mavilein mavilein added the tech/engines Issue for tech Engines. label Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech/engines/migration engine Issue in the Migration Engine tech/engines Issue for tech Engines.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants