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 rename support for MySQL versions below 5.7 (and MariaDB) #321

Merged

Conversation

nover
Copy link
Contributor

@nover nover commented Jul 13, 2017

This PR adds support for renaming indices on MySQL versions below 5.7 and MariaDB

  • ServerVersion updated to determine whether renaming is supported
  • MySqlMigrationsSqlGenerator updated to support RENAME and DROP+CREATE
  • Added tests for rename support on MySQL 5.6 and 5.7
  • Other small changes to allow unit-testing the rename support

one caveat if the index being renamed is unique and it is not version 5.7 I can't se a way to determine whether the index being changed is unique or not, hence it will DROP and create a non unique index.

Relates to #320

internal protects as well as private but now we can use the ctor
from the unit-tests
Required a small change in the get SqlGenerator where a faked
IMySqlOptions is injected with the correct MySQL version.
Right now it only overrides the Rename Index test to test that the
generator creates proper statements
Using the ServerVersion flag SupportsRenameIndex.
@caleblloyd
Copy link
Contributor

one caveat if the index being renamed is unique and it is not version 5.7 I can't se a way to determine whether the index being changed is unique or not, hence it will DROP and create a non unique index

I'll see what I can do about that

@caleblloyd caleblloyd merged commit 7a1cee9 into PomeloFoundation:master Jul 13, 2017
@caleblloyd
Copy link
Contributor

Thanks for the PR! The MySQL 5.6 test is especially nice!

@nover
Copy link
Contributor Author

nover commented Jul 13, 2017

Fantastic :) I was poking around in the IModel that is injected which does allow to find all indexes defined on the entity, but I couldn't find an index name and gave up.

I'll create a version of this for 1.1 as well.

@nover nover deleted the feature/rename-index-flag branch July 13, 2017 12:21

builder.Append("ALTER TABLE ")
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema))
.Append(" CREATE INDEX ")
Copy link
Contributor

Choose a reason for hiding this comment

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

I just realized this won't work; it doesn't define any column names. I'm re-working this function so that it can detect UNIQUE, BTREE, etc. using the SHOW CREATE TABLE logic anyways though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yeah, you're completely right, my bad. Disregard #322 then.

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