You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Both of our MySQL lexers, the new mysql-parser lexer and the mysql-on-sqlite lexer that the driver still uses, read some queries differently from MySQL. Some valid queries didn't parse, and some invalid ones did.
This change fixes these cases, so both lexers now read them like MySQL's own lexer:
t.1, t.1st, and t.1e5 are qualified names. Before, .1 was read as a decimal number.
A keyword right before a dot and a name is an identifier, so order.id works, while @@session.sql_mode keeps its meaning.
A name right after a dot is always an identifier, so t.x'41' is the column x followed by a string, not a hex literal.
A dot that ends a number doesn't make the next word a name, so SELECT 1.FROM dual now parses.
The name after @@ can't be empty, so SELECT @@ version and SELECT @@'version' are now syntax errors. It's also never a number or a literal, so @@123 is a variable name.
A dot after a quoted name or a space still starts a number, as in MySQL. All cases were checked against MySQL 8.4, and one more query from the MySQL server test corpus now parses.
The driver now stores CHECK and DEFAULT expressions without spaces around dots and after @@. It parses these expressions again to create the SQLite table. With spaces, t . order contains the ORDER keyword, and @@ max_connections has an empty variable name. Before, a constraint like CHECK (t.order > 0) failed with an internal error.
The lexers now track a state that each token sets for the next one, like MySQL's next_state. The states use MySQL's names, written out in full (for example, STATE_IDENTIFIER_SEPARATOR for MY_LEX_IDENT_SEP), so the code is easy to compare with sql_lex.cc. Lexing speed stays within noise of trunk, with and without opcache and JIT.
The PR also applies smaller improvements from the review of the new lexer:
Removes comments that only repeated the code or described its history.
Writes the backslash pattern in token unescaping as a plain literal.
Keeps the unquoted identifier characters in one shared constant.
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
Walkthrough
The MySQL lexers now use scanner states to tokenize qualified names and system-variable names. The SQLite expression serializer changes spacing around selected tokens. New tests cover tokenization, parsing, and SQLite schema expressions.
Changes: MySQL tokenization and expression round-tripping
Layer / File(s)
Summary
Qualified-name and system-variable tokenization packages/mysql-parser/src/class-wp-mysql-lexer.php, packages/mysql-on-sqlite/src/mysql/class-wp-mysql-lexer.php, packages/mysql-parser/src/class-wp-mysql-token.php, packages/mysql-parser/tests/*, packages/mysql-on-sqlite/tests/mysql/*
Both lexers use scanner state to distinguish qualified-name parts, keywords, numbers, and names following @@. Tests cover tokenization, parse acceptance and rejection, and an additional MySQL parser-suite query. The token backslash-removal pattern is now written directly.
Expression serialization and SQLite coverage packages/mysql-on-sqlite/src/sqlite/class-wp-sqlite-information-schema-builder.php, packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php
Expression serialization omits spaces around parentheses and dots and after @@. Tests cover qualified references, CHECK constraints, and DEFAULT or CHECK expressions containing system variables.
WordPress/sqlite-database-integration#562 — The changes serialize CHECK expressions for reparsing and test that qualified CHECK constraints remain in the rendered schema.
No concrete issue is established by the supplied evidence. Merge after the normal test checks.
Pre-merge checks: 4 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly identifies the main change: correcting lexer behavior. The phrase “small improvements” is broad, but it does not obscure the primary change.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 14 functions across 8 files. (1 skipped: 1 too large.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
PHPStan (2.3.1)
PHPStan reported 5 non-file analysis error(s); analysis is incomplete
PHPMD could not process this file (exit code 255): 0/1 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0%
1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 2097152 bytes) in phar:///usr/bin/phpmd/vendor/pdepend/pdepend/src/Util/Cache/Driver/FileCacheDriver.php on line 183
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
MySQL reads "t.1", "t.1st", and "order.id" as qualified names, but both
lexers failed on them: they scanned ".1" as a decimal number, and they
emitted "order" as the ORDER keyword.
Follow MySQL's lexer, where a token sets a state for the next one
("next_state" in sql/sql_lex.cc):
- A name followed by "." and a name character sets
STATE_IDENTIFIER_SEPARATOR. The "." then separates qualified name
parts, even before digits ("t.1"), and a keyword before it is an
identifier ("order.id").
- A "." token followed by a name character sets STATE_IDENTIFIER_START.
The name after it is then always an identifier ("t.order", "t.1"),
even when it looks like a literal prefix ("x" in "t.x'41'").
- "@@" followed by a name character sets STATE_IDENTIFIER_OR_KEYWORD.
The name after it is then an identifier or a keyword, never a number
or a literal ("@@123"), so system variable scopes like
"@@session.sql_mode" stay keywords.
In the last two states, the lexer reads the name before any other
scanning, as MySQL's lex_one_token() does.
A "." that ends a number sets no state, so "1.FROM dual" is a number and
the FROM keyword, as in MySQL. A "." after a quoted name or a space still
starts a number. The lexing speed stays within noise of trunk.
The fix applies to the mysql-parser lexer and to the mysql-on-sqlite lexer
that the driver still uses. The MySQL server corpus query that covers this
case now parses in both, and driver tests run such queries end to end.
See: https://github.com/mysql/mysql-server/blob/mysql-8.4.10/sql/sql_lex.cc
The driver stores CHECK and DEFAULT expressions by joining their tokens
with spaces, and it parses them again to create the SQLite table. With
spaces, "t . order" and "order . id" contain the ORDER keyword, and
"t . 1" contains a number, so the stored text no longer parses. For
example, "CHECK (t.order > 0)" failed with an internal error, while
MySQL accepts it.
Join the tokens around "." without whitespace, as the serializer already
does for parentheses, and list each of these token types once.
MySQL reads the name after "@@" in its MY_LEX_IDENT_OR_KEYWORD state,
which fails on an empty name. Only a backtick skips that state. So
"SELECT @@ version" and "SELECT @@'version'" are syntax errors in MySQL,
while "SELECT @@`version`" works. Both lexers accepted all of them.
Treat "@@" followed by neither a name character nor a backtick as
invalid input, as MySQL does.
The driver stores CHECK and DEFAULT expressions by joining their tokens
with spaces, and it parses them again on ALTER TABLE. Join "@@" and the
variable name without a space, so that "CHECK (a < @@max_connections)"
isn't stored as "a < @@ max_connections", which no longer parses. MySQL
rejects variables in these expressions, but the driver accepts them.
See: https://github.com/mysql/mysql-server/blob/mysql-8.4.10/sql/sql_lex.cc
## Release `3.1.3`
Version bump and changelog update for release `3.1.3`.
**Changelog draft:**
* Fix `BINARY` and `VARBINARY` column defaults
([#596](#596))
* Quote SQLite identifiers in `LOCK TABLES`, `SHOW TABLES`, and compound
primary key indexes
([#599](#599))
* Name lexer tokens after the MySQL grammar and default to MySQL 8.4.10
([#607](#607))
* Fix MySQL lexer correctness issues
([#584](#584))
* Require MySQL proxy authentication and default to loopback
([#540](#540))
* Speed up PHPUnit and avoid E2E setup timeouts
([#579](#579))
* Fix reserved prefix checks for case variants and foreign keys
([#580](#580))
**Full changelog:**
v3.1.2...release/v3.1.3
## Next steps
1. **Review** the changes in this pull request.
2. **Push** any additional edits to this branch (`release/v3.1.3`).
3. **Merge** this pull request to complete the release.
Merging will automatically build the plugin ZIP, create a [GitHub
release](https://github.com/WordPress/sqlite-database-integration/releases),
and deploy to
[WordPress.org](https://wordpress.org/plugins/sqlite-database-integration/).
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved handling of binary column defaults and SQLite identifier
quoting.
* Corrected MySQL lexer tokens, defaults, and lexer behavior.
* Updated MySQL proxy authentication and loopback defaults, test timing,
and reserved-prefix checks.
* **Release**
* Updated the plugin and driver version to 3.1.3.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JanJakes
changed the title
Both of our MySQL lexers, the new
mysql-parserlexer and themysql-on-sqlitelexer that the driver still uses, read some queries differently from MySQL. Some valid queries didn't parse, and some invalid ones did.This change fixes these cases, so both lexers now read them like MySQL's own lexer:
t.1,t.1st, andt.1e5are qualified names. Before,.1was read as a decimal number.order.idworks, while@@session.sql_modekeeps its meaning.t.x'41'is the columnxfollowed by a string, not a hex literal.SELECT 1.FROM dualnow parses.@@can't be empty, soSELECT @@ versionandSELECT @@'version'are now syntax errors. It's also never a number or a literal, so@@123is a variable name.A dot after a quoted name or a space still starts a number, as in MySQL. All cases were checked against MySQL 8.4, and one more query from the MySQL server test corpus now parses.
The driver now stores CHECK and DEFAULT expressions without spaces around dots and after
@@. It parses these expressions again to create the SQLite table. With spaces,t . ordercontains theORDERkeyword, and@@ max_connectionshas an empty variable name. Before, a constraint likeCHECK (t.order > 0)failed with an internal error.The lexers now track a state that each token sets for the next one, like MySQL's
next_state. The states use MySQL's names, written out in full (for example,STATE_IDENTIFIER_SEPARATORforMY_LEX_IDENT_SEP), so the code is easy to compare withsql_lex.cc. Lexing speed stays within noise of trunk, with and without opcache and JIT.The PR also applies smaller improvements from the review of the new lexer:
Follows up on #429.
Summary by CodeRabbit
@@while preserving support for backtick-quoted names.