Skip to content

Conversation

@JanJakes
Copy link
Member

@JanJakes JanJakes commented Jul 10, 2025

This PR reverts #203, where providing the DB_NAME was made optional, and implements additional checks for the correctness of the database name.

  • Implements the INFORMATION_SCHEMA.SCHEMATA table to store the database name (allowing for at most one user database).
  • Checking whether the current session database name is correct.
  • Migration path from older versions.
  • Implementing SHOW DATABASES.

@JanJakes JanJakes force-pushed the db-name-check branch 2 times, most recently from 755b3b8 to af223fe Compare July 10, 2025 13:10
@JanJakes
Copy link
Member Author

@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.

JanJakes added a commit to Automattic/wp-cli-sqlite-command that referenced this pull request Jul 11, 2025
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.
@JanJakes
Copy link
Member Author

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 information_schema.schemata and SHOW DATABASES. I will update it to store the DB name there; it will be more MySQL-native and future-proof (e.g., for multi-DB support). We can now have the limitation that only information_schema and DB_NAME can be stored there.

@bgrgicak bgrgicak self-requested a review July 11, 2025 10:13
@JanJakes JanJakes force-pushed the db-name-check branch 2 times, most recently from d6e3050 to 180ccb8 Compare July 11, 2025 13:00
JanJakes added 2 commits July 11, 2025 15:03
This also moves database name handling from a global variable
to the INFORMATION_SCHEMA.SCHEMATA table.
@JanJakes JanJakes changed the title Database name verification Add support for INFORMATION_SCHEMA.SCHEMATA, SHOW DATABASES, and verify DB names Jul 11, 2025
@JanJakes JanJakes changed the title Add support for INFORMATION_SCHEMA.SCHEMATA, SHOW DATABASES, and verify DB names Implement INFORMATION_SCHEMA.SCHEMATA, SHOW DATABASES; verify DB names Jul 11, 2025
@JanJakes
Copy link
Member Author

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 information_schema.schemata and SHOW DATABASES. I will update it to store the DB name there; it will be more MySQL-native and future-proof (e.g., for multi-DB support). We can now have the limitation that only information_schema and DB_NAME can be stored there.

I ended up changing this to:

  • Implementing the INFORMATION_SCHEMA.SCHEMATA table to store the database name (allowing for at most one user database).
  • Checking whether the current session database name is correct.
  • Migration path from older versions.
  • Implementing SHOW DATABASES.

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',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this collation?

Copy link
Member Author

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.

@adamziel adamziel merged commit e49c578 into develop Jul 14, 2025
12 checks passed
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.

4 participants