Database configuration and migration

[JanJakes]

This PR extracts existing SQLite database configuration to a WP_SQLite_Configurator and implements WP_SQLite_Information_Schema_Reconstructor with the ability to backfill missing table data in information schema.

How it works:

1. The configurator checks the SQLite driver version against a version stored in the database, and if the driver is ahead, it will execute the database (re)configuration (see WP_SQLite_Configurator::ensure_database_configured()), while acquiring an EXCLUSIVE lock to prevent race conditions (multiple requests triggering the same migration).
2. When the configurator runs, it ensures that any internal tables are created (global variables, information schema), and then it proceeds to the WP_SQLite_Information_Schema_Reconstructor.
3. The reconstructor then compares existing SQLite tables against tables recorded in the information schema, backfills those that are missing, and removes those that no longer exist.
4. To backfill a table when in WordPress, and it is a WordPress table, the definition is taken from wp_get_db_schema().
5. To backfill a non-WordPress table, the information schema data is derived from _mysql_data_types_cache, if it exists, otherwise directly from the SQLite schema using SQLite column affinity rules.

[adamziel]

Great start! And also a good idea to use a cascade of data sources for backfilling. There's some more way to go, though — I've commented on a bunch of potential issues

[adamziel]

For the purposes of this PR it's lovely. Thank you for all the thorough escaping!

Future-wise, we should start a separate discussion about query formatting. Having two formatting systems is not ideal, even if only because the sprintf() caller may accidentally pull an extra ? into the query. CC @dmsnell

That being said, I wouldn't worry about that too much here. We have to stop somewhere and that seems like a reasonable boundary.

[adamziel]

What do you think about extends \PDO as a way of enforcing the interface? Then, we can preface this with if(!class_exists('\PDO')) { class \PDO {} } to prevent fatal errors on PDO-less installations.

[adamziel]

I also wonder about clarity. Since this is SQLite-specific, in the future we could have a separate WP_MySQL_Over_SQLite_Connection class that will expect a different SQL dialect and would expose MySQL-specific quote and quote_identifier implementation. Nothing actionable here, I'm just thinking out loud.

[JanJakes]

@adamziel My thinking here was making an "internal" wrapper of PDO\SQLite or SQLite3 (or both) for connection to the SQLite database. The part that will need to expose a unified API (e.g., PDO-like), is the current WP_SQLite_Driver, I think.

But I see making this adhere to the PDO API may make be useful as well, especially to avoid reinventing the APIs.

As for the clarity of the APIs, maybe we could use the DSN prefix as PDO does? Something like:

$connection = new WP_PDO('mysql:host=localhost;dbname=wordpress'); // MySQL
$connection = new WP_PDO('sqlite:dbname=wordpress'); // SQLite -- this is the one we're discussing now
$connection = new WP_PDO('mysql-on-sqlite:dbname=wordpress'); // MySQL on SQLite -- this is the new driver
$connection = new WP_PDO('pgsql:host=localhost;dbname=wordpress'); // Postgres
$connection = new WP_PDO('pgsql-on-sqlite:dbname=wordpress'); // Postgres on SQLite

[adamziel]

At first, it felt wrong, but the more I think about it the more I like it. It's the most connection interface we can possibly offer. Really nice idea. If I'm to nitpick, maybe I'd do WP_PDO::connect() instead of new WP_PDO just to return the correct class instance right away and save on WP_PDO proxy where each method just calls $this->actual_driver->method()

[adamziel]

This is looking really good. It seems like the only blocking thing is string escaping in the DEFAULT part of CREATE TABLE and there may be an easy way out of that problem. Is it possible we're almost there?! 🤞

[JanJakes]

@adamziel This should be ready for a final review now. There are some comments that may require moving to separate tickets, but for the core migration functionality, I'd consider this complete.

[adamziel]

Beautiful work @JanJakes! Thank you for bearing with my nitpicks and persevering on this journey ❤️ I think this PR is good to go. Was this the last missing bit before enabling the new driver by default? If yes, this marks a profound milestone. We should do a P2 announcement, encourage folks to try to use and break the new driver, and ask for their overall feedback.

@wojtekn once we bundle a new version of the plugin, would you try it with a few Studio sites before a wider rollout? Better safe than sorry.

[wojtekn]

@wojtekn once we bundle a new version of the plugin, would you try it with a few Studio sites before a wider rollout? Better safe than sorry.

Does the plugin gracefully support existing sites? Does it support a migration path? Should we implement a migration path in Studio?

[JanJakes]

Does the plugin gracefully support existing sites? Does it support a migration path? Should we implement a migration pat in Studio?

With these changes, it does support all of that and there is no need to implement anything specific in Studio, apart from using define( 'WP_SQLITE_AST_DRIVER', true );, while the feature flag is still in place. We can also consider setting it to true by default now.

This was not tagged yet, so we may coordinate on that. We'll definitely need to test it more thoroughly with existing sites.

[wojtekn]

Thanks for confirming that @JanJakes, that sounds great.