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/sqlite import export support #259

Closed
wants to merge 22 commits into from

Conversation

jeroenpf
Copy link

@jeroenpf jeroenpf commented Jul 19, 2024

This PR introduces SQLite support for the wp db import and wp db export commands. This is to support WordPress sites that are running the SQLite database integration plugin.

When executing an import or export command, we check if the SQLite database integration plugin is installed and activated. If it is activated, we try to import MySQL dumps into the SQLite database or export a MySQL dump from the SQLite database.

As of now, we only support importing from and exporting to MySQL dumps.

Additionally, while I was working on this PR i encountered various issues in the SQLite database integration plugin which I've addressed and are now merged into the develop branch of the plugin's repo. For the import and export commands to work properly, you need to use the current develop branch until the next release is out. The commands heavily rely on the plugin.

Tests

The previous state of the Behat tests is that most of them fail when the tests run with SQLite as the database type. With the changes in this PR the import and export related feature tests no longer fail. For other commands, tests will still fail.

How to test

Prerequisites

Steps

  • Run wp db export test.sql and verify that a dump with the filename test.sql has been created.
  • Try to import that dump into a MySQL WordPress site using the wp db import command.
  • Verify that importing the dump went through successfully.
  • Obtain a dump for an existing MySQL powered WordPress site and then run wp db import <dump file> for a SQLite powered WordPress site. Verify that no unexpected errors occurred and that the data was successfully imported.

Known issues

  • A yet-to-be-released version of the SQLite plugin will be required for imports and exports to work properly on SQLite sites. Use the develop branch for now.
  • No conditional comments from the original dump are preserved. This includes information about SQL modes and other metadata that MySQL might normally use when importing a SQL dump. Therefore directly importing exports into a MySQL database will likely fail in many cases.
  • There are a large number MySQL statements for which there is no equivalent in SQLite or that are either not covered or broken in the SQLite integration plugin. These statements will trigger warnings when running the import command and may result in partially imported data. Examples are functions, procedures, table locks, foreign keys, update/delete constraints and many more.

@jeroenpf jeroenpf requested a review from a team as a code owner July 19, 2024 10:54
@jeroenpf jeroenpf marked this pull request as draft July 19, 2024 10:56
@adamziel
Copy link

adamziel commented Jul 23, 2024

Good call using stream processing!

If WP_SQLite_Export used $wpdb, there would be no need the same code would work for MySQL exports. What are the advantages of directly using the internal APIs such as WP_SQLite_Translator? The method signatures will likely change when we migrate to a complete grammar translator.

also cc @brandonpayton

@jeroenpf
Copy link
Author

Good call using stream processing!

If WP_SQLite_Export used $wpdb, there would be no need the same code would work for MySQL exports. What are the advantages of directly using the internal APIs such as WP_SQLite_Translator? The method signatures will likely change when we migrate to a complete grammar translator.

also cc @brandonpayton

The main reason for not using wpdb in either import or export is that the commands currently run without needing to load WP. Therefore wpdb is not available and would actually cause the commands to fail if WordPress is not (properly) installed. This is especially true for imports. The commands currently use MySQL directly, without going through WordPress and it was my thinking that it would be best to keep it like this for the SQLite export/import as well.

IMHO, it would be great if the SQLite translations part would be a package on its own so it can be used in the plugin but also anywhere else, without relying on WP.

@jeroenpf jeroenpf marked this pull request as ready for review July 25, 2024 11:09
@jeroenpf jeroenpf changed the title [WIP] Add/sqlite import export support Add/sqlite import export support Jul 25, 2024
@@ -75,23 +75,23 @@ Feature: Query the database with WordPress' MySQL config
Scenario: MySQL defaults are available as appropriate with --defaults flag
Given a WP install

When I try `wp db query --defaults --debug`
When I try `"select 1" | wp db query --defaults --debug`
Copy link

Choose a reason for hiding this comment

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

What is the context of this change?

Copy link
Author

Choose a reason for hiding this comment

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

Test command expects STDIN, none was given so it would cause the command to freeze indefinitely and the test would be stuck (at least locally).

"""
Success: Imported from 'STDIN'.
"""
When I run `wp db export wp_cli_test.sql`
Copy link

Choose a reason for hiding this comment

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

Nice catch and fix. I see this test was there for 5 years. How did it even work before?

Copy link
Author

Choose a reason for hiding this comment

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

Maybe it depends on the environment. We both experienced this issue locally but the CI seems to be fine with it.

@@ -604,6 +607,13 @@ public function export( $args, $assoc_args ) {
WP_CLI::error( 'Porcelain is not allowed when output mode is STDOUT.' );
}

// Check if SQLite is enabled and use it if it is.
if ( Export::get_sqlite_plugin_version() ) {
Copy link

Choose a reason for hiding this comment

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

Should we check for a minimal supported version of the SQLite plugin and fail with the error if it's older? Or do we rely on the logic inside load_dependencies(), which is part of import and export classes?

Copy link
Author

Choose a reason for hiding this comment

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

At the moment we check the version of the SQLite plugin when we load the dependencies. We can do this here as well. I assume you still want it to trigger the same error right?

@wojtekn
Copy link

wojtekn commented Jul 29, 2024

@jeroenpf it works well when importing my MySQL dump to a site that uses SQLite. The output nicely informs about statements that couldn't be executed:

% vendor/bin/wp db import --path=/my-sqlite-site/ /my-mysql-dump.sql
Warning: Could not execute statement: LOCK TABLES `wp_commentmeta` WRITE
Warning: Could not execute statement: UNLOCK TABLES
[...]
Success: Imported from '/my-mysql-dump.sql'.

@wojtekn
Copy link

wojtekn commented Jul 29, 2024

@jeroenpf I tested export, too. It worked, but I have some questions:

  1. What do you think about adding comments with meta details like those we can find in MySQL dump, e.g., file header or table header, and some empty lines between queries for readability?
--
-- Table structure for table `wp_commentmeta`
--
  1. Are we able to preserve engine, auto-increment value, and charset details?
  2. Is there a reason why SQLite dump code produces one insert per line instead of batching multiple records in one insert?
  3. It looks like the new SQLite dump relies on new lines as is, but the MySQL dump encodes characters, so each insert is kept on one line. Is there any reason not to follow a similar approach?

@jeroenpf
Copy link
Author

jeroenpf commented Jul 29, 2024

@jeroenpf I tested export, too. It worked, but I have some questions:

  1. What do you think about adding comments with meta details like those we can find in MySQL dump, e.g., file header or table header, and some empty lines between queries for readability?
--
-- Table structure for table `wp_commentmeta`
--
  1. Are we able to preserve engine, auto-increment value, and charset details?
  2. Is there a reason why SQLite dump code produces one insert per line instead of batching multiple records in one insert?
  3. It looks like the new SQLite dump relies on new lines as is, but the MySQL dump encodes characters, so each insert is kept on one line. Is there any reason not to follow a similar approach?
  1. Sure, sounds like a good idea. I will add it.
  2. Some notes:
    • We do not preserve the engine.
    • We can add the auto-increment value (I think) by getting the value from the sqlite_sequence table. I created an issue for this in the SQLite plugin.
    • As for the charset, SQLite only supports UTF-8 and UTF-16 - we can set that as the charset in the create statements. This is however an important point to consider when importing from a dump that is using different charsets. Another thing to consider is the collation. MySQL supports many more charsets and they can each have different collations and that can affect how data is sorted and compared. These are more complex issues and they need to be addressed in the SQLite integration plugin.
  3. MySQL does not batch inserts by default when you do a mysqldump so I followed the default behaviour. We can add an option to allow for batch inserts but i'd do that separately.
  4. I did not escape newlines because either way would be fine but we can escape newline characters so that the entire statement will appear on a single line. I will look into this.

Copy link

@sejas sejas left a comment

Choose a reason for hiding this comment

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

Great work @jeroenpf !

I've been able to export and import a SQL dump from a to a SQLite site.
The only thing I've found is that I'm not able to import the dump into my production site. I got some SQL syntax errors.

Export successfully

❯ vendor/bin/wp db export ~/Downloads/export-db.sql --path=/Users/macbookpro/Studio/my-prime-website
Success: Export complete. File written to /Users/macbookpro/Downloads/export-db.sql

Here is the exported SQL file:
export-db.sql.zip

Import Successfully

❯ vendor/bin/wp db import ~/Downloads/export-db.sql --path=/Users/macbookpro/Studio/my-wordpress-website
Warning: Could not execute statement: CREATE TABLE `wp_usermeta` (`umeta_id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,`user_id` bigint(20) unsigned NOT NULL DEFAULT '0',`meta_key` varchar(255) DEFAULT NULL,`meta_value` longtext DEFAULT ,PRIMARY KEY (`umeta_id`),KEY `meta_key` (`meta_key`),KEY `user_id` (`user_id`))
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (1,1,'nickname','admin')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (2,1,'first_name','')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (3,1,'last_name','')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (4,1,'description','')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (5,1,'rich_editing','true')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (6,1,'syntax_highlighting','true')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (7,1,'comment_shortcuts','false')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (8,1,'admin_color','fresh')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (9,1,'use_ssl',0)
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (10,1,'show_admin_bar_front','true')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (11,1,'locale','')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (12,1,'wp_capabilities','a:1:{s:13:"administrator";b:1;}')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (13,1,'wp_user_level',10)
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (14,1,'dismissed_wp_pointers','')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (15,1,'show_welcome_panel',1)
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (16,1,'session_tokens','a:2:{s:64:"33d0e4df59cee114bef8a9b9d45b65800d7fa28ac260ea558458900d9ac550c9";a:3:{s:10:"expiration";i:1722428987;s:2:"ip";s:9:"127.0.0.1";s:5:"login";i:1722256187;}s:64:"a88f8a5166a76a1a3f932427ba3775ffdd7bdd443c857c98be9d7d8b731534fa";a:3:{s:10:"expiration";i:1722433171;s:2:"ip";s:9:"127.0.0.1";s:5:"login";i:1722260371;}}')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (17,1,'wp_dashboard_quick_press_last_post_id',5)
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (18,1,'community-events-location','a:1:{s:2:"ip";s:9:"127.0.0.0";}')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (19,1,'wp_user-settings','deleted')
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (20,1,'wp_user-settings-time',1722260731)
Warning: Could not execute statement: INSERT INTO `wp_usermeta` VALUES (21,1,'wp_persisted_preferences','a:3:{s:4:"core";a:1:{s:26:"isComplementaryAreaVisible";b:1;}s:14:"core/edit-post";a:1:{s:12:"welcomeGuide";b:0;}s:9:"_modified";s:24:"2024-07-29T13:46:47.402Z";}')
Warning: Could not execute statement: CREATE TABLE `wp_termmeta` (`meta_id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,`term_id` bigint(20) unsigned NOT NULL DEFAULT '0',`meta_key` varchar(255) DEFAULT NULL,`meta_value` longtext DEFAULT ,PRIMARY KEY (`meta_id`),KEY `meta_key` (`meta_key`),KEY `term_id` (`term_id`))
Warning: Could not execute statement: CREATE TABLE `wp_commentmeta` (`meta_id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,`comment_id` bigint(20) unsigned NOT NULL DEFAULT '0',`meta_key` varchar(255) DEFAULT NULL,`meta_value` longtext DEFAULT ,PRIMARY KEY (`meta_id`),KEY `meta_key` (`meta_key`),KEY `comment_id` (`comment_id`))
Warning: Could not execute statement: CREATE TABLE `wp_postmeta` (`meta_id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,`post_id` bigint(20) unsigned NOT NULL DEFAULT '0',`meta_key` varchar(255) DEFAULT NULL,`meta_value` longtext DEFAULT ,PRIMARY KEY (`meta_id`),KEY `meta_key` (`meta_key`),KEY `post_id` (`post_id`))
Warning: Could not execute statement: INSERT INTO `wp_postmeta` VALUES (1,2,'_wp_page_template','default')
Warning: Could not execute statement: INSERT INTO `wp_postmeta` VALUES (2,3,'_wp_page_template','default')
Warning: Could not execute statement: INSERT INTO `wp_postmeta` VALUES (3,6,'_edit_lock','1722263553:1')
Warning: Could not execute statement: INSERT INTO `wp_postmeta` VALUES (4,8,'_wp_attached_file','2024/07/image.png')
Warning: Could not execute statement: INSERT INTO `wp_postmeta` VALUES (5,8,'_wp_attachment_metadata','a:6:{s:5:"width";i:3000;s:6:"height";i:2000;s:4:"file";s:17:"2024/07/image.png";s:8:"filesize";i:9033592;s:5:"sizes";a:6:{s:6:"medium";a:5:{s:4:"file";s:17:"image-300x200.png";s:5:"width";i:300;s:6:"height";i:200;s:9:"mime-type";s:9:"image/png";s:8:"filesize";i:141135;}s:5:"large";a:5:{s:4:"file";s:18:"image-1024x683.png";s:5:"width";i:1024;s:6:"height";i:683;s:9:"mime-type";s:9:"image/png";s:8:"filesize";i:1391556;}s:9:"thumbnail";a:5:{s:4:"file";s:17:"image-150x150.png";s:5:"width";i:150;s:6:"height";i:150;s:9:"mime-type";s:9:"image/png";s:8:"filesize";i:55537;}s:12:"medium_large";a:5:{s:4:"file";s:17:"image-768x512.png";s:5:"width";i:768;s:6:"height";i:512;s:9:"mime-type";s:9:"image/png";s:8:"filesize";i:820165;}s:9:"1536x1536";a:5:{s:4:"file";s:19:"image-1536x1024.png";s:5:"width";i:1536;s:6:"height";i:1024;s:9:"mime-type";s:9:"image/png";s:8:"filesize";i:2884061;}s:9:"2048x2048";a:5:{s:4:"file";s:19:"image-2048x1365.png";s:5:"width";i:2048;s:6:"height";i:1365;s:9:"mime-type";s:9:"image/png";s:8:"filesize";i:4758990;}}s:10:"image_meta";a:12:{s:8:"aperture";s:1:"0";s:6:"credit";s:0:"";s:6:"camera";s:0:"";s:7:"caption";s:0:"";s:17:"created_timestamp";s:1:"0";s:9:"copyright";s:0:"";s:12:"focal_length";s:1:"0";s:3:"iso";s:1:"0";s:13:"shutter_speed";s:1:"0";s:5:"title";s:0:"";s:11:"orientation";s:1:"0";s:8:"keywords";a:0:{}}}')
Warning: Could not execute statement: INSERT INTO `wp_posts` VALUES (6,1,'2024-07-29 13:47:16','2024-07-29 13:47:16','<!-- wp:paragraph --><p>Yay it means it works " '' quotes!\''\"</p><!-- /wp:paragraph --><!-- wp:paragraph --><p></p><!-- /wp:paragraph --><!-- wp:paragraph --><p>This is a picture of Madrid</p><!-- /wp:paragraph --><!-- wp:image {"id":8,"sizeSlug":"large","linkDestination":"none"} --><figure class="wp-block-image size-large"><img src="http://localhost:8882/wp-content/uploads/2024/07/image-1024x683.png" alt="" class="wp-image-8"/></figure><!-- /wp:image -->','First city '' yay','','publish','closed','closed','','first-city','','','2024-07-29 14:32:50','2024-07-29 14:32:50','',0,'http://localhost:8882/?post_type=city&#038
Warning: Could not execute statement: p=6',0,'city','',0);INSERT INTO `wp_posts` VALUES (7,1,'2024-07-29 13:46:45','2024-07-29 13:46:45','{"version": 3, "isGlobalStylesUserThemeJSON": true }','Custom Styles','','publish','closed','closed','','wp-global-styles-twentytwentyfour','','','2024-07-29 13:46:45','2024-07-29 13:46:45','',0,'http://localhost:8882/?p=7',0,'wp_global_styles','',0);INSERT INTO `wp_posts` VALUES (8,1,'2024-07-29 13:47:11','2024-07-29 13:47:11','','image','','inherit','open','closed','','image','','','2024-07-29 13:47:11','2024-07-29 13:47:11','',6,'http://localhost:8882/wp-content/uploads/2024/07/image.png',0,'attachment','image/png',0);
Success: Imported from '/Users/macbookpro/Downloads/export-db.sql'.

Import to production site using MySQL failed.

The wp db import command I've used was the original/production wp-cli, and not from this branch.

[email protected]:~/htdocs/wp-content$ wp db import export-db.sql
ERROR 1064 (42000) at line 20 in file: 'export-db.sql': You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '
	PRIMARY KEY (`umeta_id`),
	KEY `meta_key` (`meta_key`),
	KEY `user_id` (`us...' at line 5
ERROR 1146 (42S02) at line 29 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 30 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 31 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 32 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 33 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 34 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 35 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 36 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 37 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 38 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 39 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 40 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 41 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 42 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 43 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 44 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 45 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 46 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 47 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 48 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1146 (42S02) at line 49 in file: 'export-db.sql': Table '150722608.wp_usermeta' doesn't exist
ERROR 1064 (42000) at line 51 in file: 'export-db.sql': You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '
	PRIMARY KEY (`meta_id`),
	KEY `meta_key` (`meta_key`),
	KEY `term_id` (`ter...' at line 5
ERROR 1064 (42000) at line 97 in file: 'export-db.sql': You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '
	PRIMARY KEY (`meta_id`),
	KEY `meta_key` (`meta_key`),
	KEY `comment_id` (`...' at line 5
ERROR 1064 (42000) at line 18264 in file: 'export-db.sql': You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '
	PRIMARY KEY (`meta_id`),
	KEY `meta_key` (`meta_key`),
	KEY `post_id` (`pos...' at line 5
ERROR 1146 (42S02) at line 18273 in file: 'export-db.sql': Table '150722608.wp_postmeta' doesn't exist
ERROR 1146 (42S02) at line 18274 in file: 'export-db.sql': Table '150722608.wp_postmeta' doesn't exist
ERROR 1146 (42S02) at line 18275 in file: 'export-db.sql': Table '150722608.wp_postmeta' doesn't exist
ERROR 1146 (42S02) at line 18276 in file: 'export-db.sql': Table '150722608.wp_postmeta' doesn't exist
ERROR 1146 (42S02) at line 18277 in file: 'export-db.sql': Table '150722608.wp_postmeta' doesn't exist
ERROR at line 18406 in file: 'export-db.sql': Unknown command '\"'.
ERROR 1064 (42000) at line 18406 in file: 'export-db.sql': You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '\"</p>
<!

<!
<figure class="wp-block-image size-large"><img src="http://loca/...' at line 1
Success: Imported from 'export-db.sql'.

Import failed using phpmyadmin

Screenshot 2024-07-29 at 16 18 56

Maybe I did something wrong in my test steps, or these are edge cases that are not covered yet. My original site was created with Studio and sqlite-database-integration v2.1.12.
I installed the plugin Custom Post Type UI and created some custom post types.

Comment on lines 9 to 15
if ( ! function_exists( 'do_action' ) ) {
function do_action() {} // phpcs:ignore
}

if ( ! function_exists( 'apply_filters' ) ) {
function apply_filters() {} // phpcs:ignore
}
Copy link

Choose a reason for hiding this comment

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

Is this something we can avoid by updating the SQLite database integration plugin?

Copy link
Author

Choose a reason for hiding this comment

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

yes, this particular code can now be removed since my fix in the SQLite plugin has been merged and released: WordPress/sqlite-database-integration#135

@jeroenpf
Copy link
Author

@sejas Thanks for testing. You uncovered a critical bug. I've created a PR to address it. The reason you experienced the issue is because the output of show create is invalid for NULL columns that have no default value.

Copy link

@sejas sejas left a comment

Choose a reason for hiding this comment

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

@jeroenpf , Thanks for the fix! I confirm that using the latest version of https://github.com/WordPress/sqlite-database-integration/tree/develop, the previous error has been fixed 🥳 .

I've found another bug related to escaped single quotes in the post content:

Screenshot 2024-07-30 at 12 54 39

Here is the exported SQL dump:
export-new-db.sql.zip

Steps to reproduce:

  • Create a site in Studio
  • Add a new post with any title and this paragraph: \'
  • Run vendor/bin/wp db export export-db.sql --path=PATH-TO-YOUR-STUDIO-SITE
  • Try to import the export-db.sql in PHPMyAdmin
  • Observe the error

The content Just escape a single quote \' produces this SQL query that is not correct in MySQL:

INSERT INTO `wp_posts` VALUES (6,1,'2024-07-29 13:47:16','2024-07-29 13:47:16','<!-- wp:paragraph --><p>Just escape a single quote \''</p><!-- /wp:paragraph --><!-- wp:paragraph --><p></p><!-- /wp:paragraph --><!-- wp:paragraph --><p>This is a picture of Madrid</p><!-- /wp:paragraph --><!-- wp:image {"id":8,"sizeSlug":"large","linkDestination":"none"} --><figure class="wp-block-image size-large"><img src="http://localhost:8882/wp-content/uploads/2024/07/image-1024x683.png" alt="" class="wp-image-8"/></figure><!-- /wp:image -->','First city '' yay','','publish','closed','closed','','first-city','','','2024-07-30 11:59:59','2024-07-30 11:59:59','',0,'http://localhost:8882/?post_type=city&#038;p=6',0,'city','',0);INSERT INTO `wp_posts` VALUES (7,1,'2024-07-29 13:46:45','2024-07-29 13:46:45','{"version": 3, "isGlobalStylesUserThemeJSON": true }','Custom Styles','','publish','closed','closed','','wp-global-styles-twentytwentyfour','','','2024-07-29 13:46:45','2024-07-29 13:46:45','',0,'http://localhost:8882/?p=7',0,'wp_global_styles','',0);

The probability of a user adding a escaped single quote in their content is very low and could be considered an edge case. But ideally the db export should work in any case.
Thank you!

Copy link

@sejas sejas left a comment

Choose a reason for hiding this comment

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

@jeroenpf, Thanks for the fix! I've been able to export a SQL dump from a site with SQLite and import it on a production site with MySQL 😍.
Great work! 🙌

@adamziel
Copy link

adamziel commented Aug 2, 2024

IMHO, it would be great if the SQLite translations part would be a package on its own so it can be used in the plugin but also anywhere else, without relying on WP.

Agreed! Unfortunately, we aren't there today and might not get there soon :( A glimmer of hope is that we'll likely need to use a MySQL parser, not just a tokenizer, and that might be a good excuse to start decoupled from WordPress.

As a side note, it would also be lovely to ship this export/import flow as a separate package decoupled from WP-CLI.

The main reason for not using wpdb in either import or export is that the commands currently run without needing to load WP. Therefore wpdb is not available and would actually cause the commands to fail if WordPress is not (properly) installed. This is especially true for imports. The commands currently use MySQL directly, without going through WordPress and it was my thinking that it would be best to keep it like this for the SQLite export/import as well.

What is the challenge with using $wpdb on a not (properly) installed site? WordPress installer seems to be working fine with that even though there are no tables at the beginning of the process. If your concern is a fatal error somewhere in the WordPress loading flow, then it can likely be mitigated with a drop-in plugin intercepting the execution flow, or with the shutdown handler, or with a constant.

As it stands, this PR is SQLite specific. Using $wpdb would make it just work for MySQL<->SQLite, MySQL<->MySQL, and SQLite<->SQLite even in environments without mysqldump.

I also want to stress that the https://github.com/WordPress/sqlite-database-integration plugin will change. The file structure and the API will both evolve and, as a result, break the implementation proposed in this PR. However, $wpdb will likely always remain backwards compatible.

$this->write_insert_statements( $handle, $table->name );
}

fwrite( $handle, sprintf( '-- Dump completed on %s', gmdate( 'c' ) ) );
Copy link

Choose a reason for hiding this comment

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

fwrite() enforces using PHP resource handles. This is a good start, and building on top of that it would be lovely to output that as a simple string for the API consumer to stream-write wherever it needs to. This would play well with the Streaming API I'm exploring for WordPress.

@wojtekn
Copy link

wojtekn commented Aug 3, 2024

What is the challenge with using $wpdb on a not (properly) installed site? WordPress installer seems to be working fine with that even though there are no tables at the beginning of the process. If your concern is a fatal error somewhere in the WordPress loading flow, then it can likely be mitigated with a drop-in plugin intercepting the execution flow, or with the shutdown handler, or with a constant.

As it stands, this PR is SQLite specific. Using $wpdb would make it just work for MySQL<->SQLite, MySQL<->MySQL, and SQLite<->SQLite even in environments without mysqldump.

@adamziel note that the current wp db import and wp db export implementations support MySQL by relying on MySQL and mysqldump tools. It makes sense to follow a similar approach to add support for SQLite.

I agree it would be great if everything worked on $wpdb, but it makes sense to work on that separately and refactor the command holistically. Our current goal is to add support for SQLite and not to refactor the tool.

Agreed! Unfortunately, we aren't there today and might not get there soon :(

This is a good reason to implement it as we have now and refactor it in the future when possible.

I also want to stress that the https://github.com/WordPress/sqlite-database-integration plugin will change. The file structure and the API will both evolve and, as a result, break the implementation proposed in this PR. However, $wpdb will likely always remain backwards compatible.

What significant changes are planned? Are you worried that updating WP CLI SQLite import and export code will be challenging to keep up with SQLite plugin changes?

@schlessera
Copy link
Member

As discussed in Slack, this approach is not a good fit for bundling into WP-CLI directly, given its dependencies.

For clarification: the database handling of WP-CLI needs to be functional independently of WordPress. The WP installation might be uninitialized, broken, or not even on the same server. That's the reason why the wp db commands for MySQL use the mysql & mysqldump binaries instead of the PHP mysqli extension or $wpdb. And if the SQLite is heading towards being more integrated with $wpdb, that is all the more reason to not rely on it for the purposes of WP-CLI.

The above will be moved into a separate plugin or third-party command package so it can be used right away where it makes sense. But SQLite functionality that is meant to be bundled with WP-CLI will need to look at a different approach.

@schlessera
Copy link
Member

I have looked at two different approaches for making SQLite compatibility work from within a separate package (as previously agreed on here):

  1. Create a package that parses a given dialect and makes transformations to it as needed, like the current SQLite plugin does. This could be worked on by extracting the logic out of the SQLite plugin and refactoring it into a more maintainable generic PHP package.
block-beta
    columns 3
    imysql[("MySQL")]
    space
    isqlite[("SQLite")]
    down1<[" "]>(down)
    space
    down2<[" "]>(down)
    block:converter("Transformer\nPackage"):3
          mtos["MySQL => SQLite"]
          space
          stom["SQLite => MySQL"]
    end
    down3<[" "]>(down)
    space
    down4<[" "]>(down)
    omysql[("MySQL")]
    space
    osqlite[("SQLite")]
Loading
  1. Create a package that can read multiple dialects into an intermediate in-memory representation and that can render the in-memory representation into different dialects. This has the advantage that it is easy to extend towards other dialects and to maintain for different versions, and provide flexible many-to-many conversions as a side-effect. I've started working on the foundation of such a package here: https://github.com/wp-cli/sql-transform. It still needs a lot of work, but I think that approach is ultimately the most promising.
block-beta
    columns 3
    imysql[("MySQL")]
    imariadb[("MariaDB")]
    isqlite[("SQLite")]
    block:converter("Transformer\nPackage"):3
    columns 3
        mysqlparser["MySQL Parser"]
        mariadbparser["MariaDB Parser"]
        sqliteparser["SQLite Parser"]
        space:3
        space inmemory("In-memory\nrepresentation") space
        space:3
        mysqlrenderer["MySQL Renderer"]
        mariadbrenderer["MariaDB Renderer"]
        sqliterenderer["SQLite Renderer"]
    end
    omysql[("MySQL")]
    omariadb[("MariaDB")]
    osqlite[("SQLite")]
    imysql --> mysqlparser
    imariadb --> mariadbparser
    isqlite --> sqliteparser
    mysqlparser --> inmemory
    mariadbparser --> inmemory
    sqliteparser --> inmemory
    inmemory --> mysqlrenderer
    inmemory --> mariadbrenderer
    inmemory --> sqliterenderer
    mysqlrenderer --> omysql
    mariadbrenderer --> omariadb
    sqliterenderer --> osqlite
Loading

@adamziel
Copy link

adamziel commented Aug 8, 2024

What significant changes are planned? Are you worried that updating WP CLI SQLite import and export code will be challenging to keep up with SQLite plugin changes?

The SQLite integration plugin may switch from token-by-token processing to AST transformations. @schlessera we seem to be working through the same problem and I'd love your input there.

@adamziel
Copy link

adamziel commented Aug 9, 2024

2. Create a package that can read multiple dialects into an intermediate in-memory representation and that can render the in-memory representation into different dialects.

@schlessera I have tangential notes on two aspects of that – I'm happy to move this discussion somewhere else to keep this PR focused.

Inputs

What would be the use-case for parsing SQLite queries?

Also, if we get different parsers for MariaDB and MySQL, then we also need different parsers for MySQL 5.7, 8.0, 8.1 etc. My hope is the same parser could handle all of these. As far as I understand, MariaDB syntax is mostly compatible with MySQL and the official list of incompatible features doesn't seem too bad at the query parsing level.

Parsing model

I don't think a Query -> Intermediate representation -> Query transpilation model is sufficient. I think we need a custom MySQL on SQLite database driver that goes beyond transpilation.

In functional terms, transpilation would be a pure function like (MySQL_Query) => SQLite_Query. However, we can only handle the general case with a function like (MySQL_Query, SQL_Connection) => () => Query_Results, meaning we also use the SQLite connection and we derive the query results not from a simple database query but from a computation that may involve a back-and-forth cascade of database queries and transformations.

Here's a few examples:

  • ALTER TABLE ADD COLUMN has no SQLite counterpart and we're running an 8-step algorithm to approximate it with other queries. This one's still a transpilation from one to many queries ((MySQL_Query) => SQLite_Query[]).
  • SHOW CREATE TABLE result can't be derived from SQLite tables at all. Most MySQL data types don't exist in SQLite, e.g. dates become strings. We keep track of the original type definitions in a separate table and pull it from there on demand to reconstruct the original CREATE TABLE query ((MySQL_Query, SQL_Connection) => () => Query_Results)
  • SQL_CALC_FOUND_ROWS and FOUND_ROWS() have no SQLite counterpart. We support it by executing an intermediate query, reading the results, and including the actual number of rows in the original query, as in SELECT 4 AS "FOUND_ROWS()". This, again, is (MySQL_Query, SQL_Connection) => () => Query_Results.

Related: SQLGlot is a SQL -> SQL transpiler build in Python. Their parser is more powerful than the one in the https://github.com/WordPress/sqlite-database-integration plugin, and yet it won't handle the MySQL -> SQLite transition for SQL_CALC_FOUND_ROWS and other MySQL features that are beyond transpilation.

@jeroenpf
Copy link
Author

We've extracted the code in this PR into a separate command for now: https://github.com/Automattic/wp-cli-sqlite-command/

I've closed the PR. Thanks to everyone for their contributions to the discussion!

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.

5 participants