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

Enum to varchar #963

Merged
merged 40 commits into from
Jun 10, 2021
Merged

Enum to varchar #963

merged 40 commits into from
Jun 10, 2021

Conversation

shlomi-noach
Copy link
Contributor

Related issue: #642

Description

Looking into -alter converting an enum into varchar. Per #642 this copies the numeric value of the enum into the varchar column, where we want the textual value.

Initial commit: test suite to confirm error.

Support a complete ALTER TABLE statement in --alter
Initial commit: towards setting up a test suite

Signed-off-by: Shlomi Noach <[email protected]>
…original table, applying AUTO_INCREMENT value onto ghost table if applicable and user has not specified AUTO_INCREMENT in alter statement
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Copying AUTO_INCREMENT value to ghost table
Generated column as part of UNIQUE (or PRIMARY) KEY
Cut-over should wait for heartbeat lag to be low enough to succeed
All MySQL DBs limited to max 3 concurrent/idle connections
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
hooks: reporting GH_OST_ETA_SECONDS. ETA as part of migration context
@timvaillancourt
Copy link
Collaborator

@shlomi-noach any concerns if I re-open this PR? I think this includes some PRs we haven't merged but this would be useful to have in this repo

@shlomi-noach
Copy link
Contributor Author

@timvaillancourt let me open a new one, since there's been more commits to this branch than are present here.

@timvaillancourt
Copy link
Collaborator

@shlomi-noach oh, thanks a lot!

We can handle getting this PR in-sync if you'd like, I think we'll just need to merge a few dependant PRs to get this one mergeable. Totally up to you, thanks!

@shlomi-noach
Copy link
Contributor Author

@timvaillancourt I see some merge conflicts locally, looks like I need to pull from upstream. Will do so tomorrow morning!

@shlomi-noach shlomi-noach reopened this May 4, 2021
@shlomi-noach
Copy link
Contributor Author

@timvaillancourt well that was short. The PR is all yours!

Signed-off-by: Shlomi Noach <[email protected]>
@timvaillancourt
Copy link
Collaborator

timvaillancourt commented May 4, 2021

@shlomi-noach thanks a lot!

This support will unblock some varchar -> enum migrations we wanted to do but were afraid we couldn't roll back ❤️🎉

@shlomi-noach
Copy link
Contributor Author

unblock some varchar -> enum

I hope you mean enum -> varchar 😛

@timvaillancourt
Copy link
Collaborator

unblock some varchar -> enum

I hope you mean enum -> varchar 😛

@shlomi-noach I think the blocked migrations are varchar -> enum (which already worked?) but we were worried we couldn't reverse the migration without trashing the data if we needed to

I believe we would have numbers instead of real varchar values if we reversed a migration to enum

@shlomi-noach
Copy link
Contributor Author

ahh, got it.

@shlomi-noach
Copy link
Contributor Author

branch updated after recent merge of downstream contribution.

@timvaillancourt
Copy link
Collaborator

This PR passed integration testing 100%, merging. Thanks @shlomi-noach!

@timvaillancourt timvaillancourt merged commit 9bc508f into github:master Jun 10, 2021
@shlomi-noach shlomi-noach deleted the enum-to-varchar branch June 10, 2021 17:47
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.

2 participants