Handle database with escapable identifiers in name #717
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In MySQL a hyphen (-) in a database name is not prohibited but requires the name to be escaped with backtick (`). The methods setConnection() and createDatabase() passed the database name unaltered which caused migrations to fail for systems with hyphens in the database name. This change wraps the database name in quoteTableName(), and also identified a couple other cases where the name should be or was inconsistently quoted.
Adding a test case to cover databases with hyphen in its name.
Looks like the TABLE_SCHEMA was being quoted already, so this double-quoted and caused some tests to fail.
|
Thank you 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the case where a MySQL database has a hyphen in it, the updated migrations in 4.3.0 cause the migration to fail with a syntax error. The previous Phinx-backed migrations did not encounter this.
Looking into the code, there were only two locations that did not escape the database name,
setConnection()andcreateDatabase(), which have been updated to use thequoteTableName()method. Additionally, there were a couple other instances where the database name was being manually quoted with backticks, so I refactored those to use thequoteTableName()method as well for consistency.Tests have been updated to explicitly create and drop a database with the configured name plus a hyphenated suffix component. All tests continue to pass. The
dropDatabase()method which runs beforecreateDatabase()in the test setup had previously had quoting hard-coded into the query, which explains why the original syntax error did not occur at that point. After refactoring, thedropDatabase()method continued to work without regression.