Remove custom parser in favor of AST parser #13

sejas · 2025-10-08T11:04:30Z

Fixes:

As suggested in WordPress/sqlite-database-integration#263 (comment) , we'll use the AST parser to split the queries from a SQL dump file. The only downside is that we are loading the whole dump into memory.

Testing Instructions

Create a blank site using Studio
It doesn't matter if the site is running or not.
Edit the file ~/Library/Application Support/Studio/server-files/sqlite-command/src/Import.php with the changes from this PR.
In Studio go to the Import/Export tab
Try multiple queries like those provided on STU-844 or WP_MySQL_Token::get_value() throws error when importing wp_aioseo_notifications SQL query WordPress/sqlite-database-integration#250, or wp_redirection_404_with-create-table.sql
Confirm that the SQL files are imported correctly.
Try importing the same SQL dump and confirm that an alert appears stating that "Create Table" couldn't be executed. This is expected because you already imported that dump and created the table.
Feel free to try other sites and queries to confirm everything works as expected.

sejas · 2025-10-08T11:08:23Z

src/Import.php

-
-		fclose( $handle );
+	protected function remove_comments( $text ) {
+		return preg_replace( '/\/\*.*?\*\/(;)?/s', '', $text );


I had to remove all the comments with a regex because the AST parser was identifying them as queries to execute, which caused it to fail. For example Error: SQLite import could not execute statement: SET @saved_cs_client = @@character_set_client */;

@sejas Ahh, this is because the /*!<number> ...*/ comments are special MySQL comments that can execute queries conditionally based on the MySQL version. Therefore, /*!40101 SET character_set_client = @saved_cs_client */; means execute this on all versions >= 4.1.1. So the part with the query being executed is correct.

But somehow, the execution fails... so let's keep a quick fix. Regexes are tricky because they can match any random string in the dump, etc. What about catching the error instead, and if it starts with SQLite import could not execute statement: SET @, then we would skip it? I would then check the root cause of the failure.

One thing I just noticed is that the statement doesn't include the leading /* for some reason 🤔 Trying just $this->assertQuery( '/*!40101 SET character_set_client = @saved_cs_client */;' ); — and this passes. Anyway, we can have a hotfix for now.

Yeah, I noticed that too. The leading /* exists in the file, but not when using the AST parser.

I removed the regex and used a basic check: ca2f090#diff-aea1542aa0e46981e70c6bfb53a15a583242580d74dfb5df25dbfe71d098757bR159-R162
I'm checking that the query that failed starts with SET and it contains */.

Nice solution! This way it will target exactly these "wrongly parsed" comments.

sejas · 2025-10-08T11:52:10Z

src/Import.php

+				$this->driver->query( $statement );
+			} catch ( Exception $e ) {
+				// Skip errors when executing SET comment statements
+				if ( 0 === strpos( $statement, 'SET ' ) && false !== strpos( $statement, '*/' ) ) {


I cannot use str_starts_with or str_contains because we support PHP 7.4

sejas · 2025-10-08T11:53:01Z

src/Import.php

+				// Skip errors when executing SET comment statements
+				if ( 0 === strpos( $statement, 'SET ' ) && false !== strpos( $statement, '*/' ) ) {
+					WP_CLI::warning( 'SQLite import SET comment statement: ' . $statement );
+					continue;
+				}


See #13 (comment)

JanJakes

Thanks for working on this!

wojtekn · 2025-10-08T12:34:24Z

@sejas I couldn't make it work for either query:

… noise in Studio alert error messages

sejas · 2025-10-08T14:32:41Z

@wojtekn , thanks for testing it.
The second error occurred because I provided only the insert query without including the create table statement. If you use the full SQL dump, it will run correctly.

About the first error, it seems the WordPress/sqlite-database-integration#250 had another error related to the encoding. I fixed it on d897a9e

sejas · 2025-10-08T14:34:19Z

Currently I found an error when importing a whole site with 185MB+ in multiple tables and it seems it exhausts the Wasm/Node memory.

Success: Imported from 'studio-backup-sql-2025-10-08-15-27-38.sql'.
Imported /var/folders/_x/rbv26n3925q_01dbs4jzd8t80000gn/T/studio_backupECIZ4S/sql/wp_actionscheduler_logs.sql in 0.729s
Importing /var/folders/_x/rbv26n3925q_01dbs4jzd8t80000gn/T/studio_backupECIZ4S/sql/wp_analytics_wp_events.sql...
Error during import of /var/folders/_x/rbv26n3925q_01dbs4jzd8t80000gn/T/studio_backupECIZ4S/sql/wp_analytics_wp_events.sql: PHP.run() failed with exit code 255. 

=== Stdout ===
 #!/usr/bin/env php
Warning - SQLite import skipped SET comment statement: SET @saved_cs_client     = @@character_set_client */;
Warning - SQLite import skipped SET comment statement: SET character_set_client = utf8mb4 */;
Warning - SQLite import skipped SET comment statement: SET character_set_client = @saved_cs_client */;
<br />
<b>Fatal error</b>:  Allowed memory size of 268435456 bytes exhausted (tried to allocate 4096 bytes) in <b>/wordpress/wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite-ast/class-wp-sqlite-driver.php</b> on line <b>3176</b><br />


=== Stderr ===
 PHP Fatal error:  Allowed memory size of 268435456 bytes exhausted (tried to allocate 4096 bytes) in /wordpress/wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite-ast/class-wp-sqlite-driver.php on line 3176

Would have bumped stat: studio-app-import=failure
Sentry Logger [log]: Captured error event `Database import failed: PHP.run() failed with exit code 255.

JanJakes · 2025-10-08T15:52:23Z

@sejas I wonder if there's a memory leak of some sorts, or if there's a single query whose AST doesn't fit twice into memory. Does any of these help?

Call unset( $ast ); just after $statement = substr( ... );.
Call gc_collect_cycles() at the end of each loop, or just after $statement = substr( ... );.

JanJakes · 2025-10-08T16:01:53Z

@sejas Oh, now I see there is file_get_contents(), unlike in the old code. Hmm, right. Doing this correctly means supporting streaming all the way. A quick fix with a basic approach could be something like

Read up to, e.g., 10MB of content.
Parse and execute all the queries.
When encountering a parse error, take new 10MB from the beginning of the failing query.
Now if it fails again, it's an error. If it passes, it was incomplete input.

(It would mean that the limit for a single query is 10MB, or whatever we set it to).

JanJakes · 2025-10-08T16:35:33Z

@sejas One last question—wasn't the encoding perhaps the original issue? The original query-parsing code seems quite solid—tracking both escaping and quotes. And it seems to process the queries from WordPress/sqlite-database-integration#250 correctly.

What if we only add the encoding fix and keep the old query parsing code? Would that work?

sejas · 2025-10-09T09:12:05Z

We have two different issues:

250 is an encoding error
263 is a parse error

We can create two different PRs for those 👍 .

As discussed on Slack, let's keep the WP-CLI parser because adding support for file streams in the AST parser seems more complex.

adamziel · 2025-10-09T14:23:02Z

Do we need the new parser for this? It seems like we want to iterate over queries from a potentially large files – maybe the tokenizer alone would suffice? We'd just split at the query boundary and presto?

JanJakes · 2025-10-09T14:27:48Z

maybe the tokenizer alone would suffice

@adamziel Yes! That's a great point. We still need to make the tokenizer support streaming, but it's likely much easier than with the parser.

sejas added 3 commits October 8, 2025 10:40

Remove custom parser in favor of AST parser

5227c4c

Add try catch instead of checking $result false value

84a3a8d

Add semi colon to comments regex

6fe2f55

sejas self-assigned this Oct 8, 2025

sejas requested a review from JanJakes October 8, 2025 11:05

sejas mentioned this pull request Oct 8, 2025

Failed to parse the MySQL query error WordPress/sqlite-database-integration#263

Closed

sejas commented Oct 8, 2025

View reviewed changes

sejas added 3 commits October 8, 2025 12:27

Add support for old driver WP_SQLite_Translator

79063a8

Update comments

e7e8570

Bypass SET comments instead of removing all the comments

ca2f090

sejas commented Oct 8, 2025

View reviewed changes

sejas requested a review from a team October 8, 2025 12:02

JanJakes approved these changes Oct 8, 2025

View reviewed changes

sejas added 2 commits October 8, 2025 13:53

Fix comment indentation

b58afac

Fix file encoding like: WordPress/sqlite-database-integration#250

d897a9e

sejas mentioned this pull request Oct 8, 2025

WP_MySQL_Token::get_value() throws error when importing wp_aioseo_notifications SQL query WordPress/sqlite-database-integration#250

Closed

Echo the skipped comments instead of using a wp-cli warning to reduce…

fdf1b63

… noise in Studio alert error messages

sejas mentioned this pull request Oct 8, 2025

Add which table is being imported and SQLite import stdout to Studio logs Automattic/studio#1861

Merged

1 task

sejas closed this Oct 9, 2025

adamziel mentioned this pull request Oct 9, 2025

WP_MySQL_Naive_Query_Stream WordPress/sqlite-database-integration#264

Draft

JanJakes mentioned this pull request Oct 10, 2025

Fix import query parsing #14

Merged

Remove custom parser in favor of AST parser #13

Remove custom parser in favor of AST parser #13

Uh oh!

Conversation

sejas commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing Instructions

Uh oh!

sejas Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

JanJakes Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

JanJakes Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

sejas Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

sejas Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

JanJakes Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

sejas Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

sejas Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JanJakes left a comment

Choose a reason for hiding this comment

Uh oh!

wojtekn commented Oct 8, 2025

Uh oh!

sejas commented Oct 8, 2025

Uh oh!

sejas commented Oct 8, 2025

Uh oh!

JanJakes commented Oct 8, 2025

Uh oh!

JanJakes commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JanJakes commented Oct 8, 2025

Uh oh!

sejas commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamziel commented Oct 9, 2025

Uh oh!

JanJakes commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sejas commented Oct 8, 2025 •

edited

Loading

sejas Oct 8, 2025 •

edited

Loading

JanJakes commented Oct 8, 2025 •

edited

Loading

sejas commented Oct 9, 2025 •

edited

Loading