-
Notifications
You must be signed in to change notification settings - Fork 52
Implement INFORMATION_SCHEMA.SCHEMATA, SHOW DATABASES; verify DB names
#213
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
Conversation
755b3b8 to
af223fe
Compare
|
@adamziel Here's the DB name enforcing logic put back & improved to check for correctness. Everyone's AFK now, so I may just merge this tomorrow. |
This ensures the DB_NAME constant from "wp-config.php" is used, and aligns these commands with similar "wp db" commands. With WordPress/sqlite-database-integration#213, the correctness of the database name will also be verified.
|
I was further thinking about this solution, and there's one thing I don't like — the use of the global variable, since MySQL global variables are not per database but per the whole instance. That made me think — what's the native way of storing database names in MySQL? It's the |
d6e3050 to
180ccb8
Compare
This also moves database name handling from a global variable to the INFORMATION_SCHEMA.SCHEMATA table.
INFORMATION_SCHEMA.SCHEMATA, SHOW DATABASES, and verify DB names
INFORMATION_SCHEMA.SCHEMATA, SHOW DATABASES, and verify DB namesINFORMATION_SCHEMA.SCHEMATA, SHOW DATABASES; verify DB names
I ended up changing this to:
I'm leaving it open for review, as this can wait for when we're back from AFKs. |
| 'UPDATE_TIME' => null, | ||
| 'CHECK_TIME' => null, | ||
| 'TABLE_COLLATION' => 'utf8mb4_general_ci', | ||
| 'TABLE_COLLATION' => 'utf8mb4_0900_ai_ci', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this collation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the default in MySQL 8+. See also the last commit separately for some more context. The upside is more correct MySQL compatibility, the downside that such exports won't work on older MySQL and MariaDB - but that's already the case with MySQL 8 as well. The default on MySQL 5 seems to be 'latin1_swedish_ci', which is not great.
This PR reverts #203, where providing the
DB_NAMEwas made optional, and implements additional checks for the correctness of the database name.