Sync with upstream trunk (2 commits) - #10
Open
batonac wants to merge 8 commits into
Open
Conversation
## What? Align SQL savepoint handling with MySQL semantics. A standalone `SAVEPOINT` in the default autocommit mode no longer opens an SQLite transaction. It is discarded immediately, so following writes use the normal `BEGIN IMMEDIATE` wrapper and commit independently. Inside explicit transactions, track active savepoint names to emulate MySQL behavior for nested savepoints, reused and case-insensitive names, `ROLLBACK TO`, `RELEASE`, and missing-savepoint error 1305. Missing-savepoint errors leave the surrounding transaction active. Add regression coverage across the MySQL-on-SQLite, concurrency, PDO API, and WordPress integration test suites. ## Why? SQLite treats a standalone `SAVEPOINT` as opening a transaction, but MySQL does not. Passing it through to SQLite created a hidden transaction on PHP versions before 8.4. Subsequent writes then either attempted a nested `BEGIN IMMEDIATE` or bypassed the normal write-locking path, depending on how the transaction state was tracked. The original Data Machine reproduction relied on SQLite-only behavior: issuing a bare `SAVEPOINT`, performing a write, and committing with `RELEASE SAVEPOINT`. Real MySQL and MariaDB discard the standalone savepoint, so the later release reports error 1305. That downstream issue is tracked in Extra-Chill/data-machine#3436. > [!NOTE] > Consumers that relied on a bare `SAVEPOINT` opening a transaction on SQLite must open an explicit transaction instead, matching MySQL behavior. Fixes WordPress#495 --------- Co-authored-by: Jan Jakeš <jan@jakes.pro>
batonac
added
upstream-sync
…keys (WordPress#500) ## Summary `SHOW CREATE TABLE` reconstructs the `PRIMARY KEY` clause from the information schema, but unlike the branch that handles all other indexes, it emitted only the quoted column names. Index prefix lengths (`SUB_PART`) and descending order (`COLLATION = 'D'`) were dropped, so a table created with `PRIMARY KEY (session_id(100))` on a `MEDIUMTEXT` column came back as `PRIMARY KEY (session_id)`. The practical impact: exports built on top of `SHOW CREATE TABLE` (for example `wp sqlite export`, which WordPress Studio uses for its push feature) produce a dump that MySQL rejects with `ERROR 1170 (42000): BLOB/TEXT column used in key specification without a key length`. The prefix is recorded correctly in `_wp_sqlite_mysql_information_schema_statistics` (`SUB_PART = 100`); the bug was emission only. Discovered through Automattic/studio#4739. Fixes WordPress#501 ## Fix The column formatting closure that already handled `SUB_PART` and `DESC` for regular keys is hoisted and shared by the `PRIMARY KEY` branch, so both paths emit identical column definitions. ## Testing - New regression test: a `PRIMARY KEY (session_id(100))` on `MEDIUMTEXT` round-trips through `SHOW CREATE TABLE` with its prefix, a composite key `PRIMARY KEY (a, b(50))` keeps the prefix only where defined, and `PRIMARY KEY (a DESC, b)` keeps its descending key part. The test fails on the previous code. - Verified against a real MySQL 8 server: its `SHOW CREATE TABLE` emits `PRIMARY KEY (`session_id`(100))` and `PRIMARY KEY (`a` DESC,`b`)` for the same tables, matching the fixed output; the previous output fails to import there with `ERROR 1170`, the fixed output imports cleanly. - Full `mysql-on-sqlite` unit suite: 875 tests, 1428750 assertions, no failures (the same 17 skipped and 2 incomplete as on trunk, PHP 8.5). - `composer run check-cs` is clean on both changed files. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Corrected `SHOW CREATE TABLE` output to preserve primary-key index prefix lengths for single-column and composite keys. - Ensured descending key parts and column definitions are represented accurately in generated table-creation statements. - Improved consistency between primary-key and secondary-index formatting. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
batonac
changed the title
## Summary This PR fixes lexer edge cases and related string handling: - **String values:** Preserve binary and single-byte charset payloads and correctly decode escaped newlines. - **Function names:** Exclude trailing whitespace from function-name token ranges under `IGNORE_SPACE`. - **Version comments:** Support six-digit versions from MySQL 8.1, preserve five-digit fallback parsing and unversioned numeric content, and recognize vertical tabs as whitespace. - **WordPress validation:** Mirror the parent `wpdb::query()` charset checks, retaining its binary-data and prevalidated-query exemptions. - **`LIKE BINARY` patterns:** Preserve raw bytes instead of producing `NULL` during GLOB conversion, and handle escaped newlines. The lexer fixes cover both PHP packages and the native extension. ## Why The token decoder's UTF-8 regex modifier caused an accidental `TypeError` for legitimate binary data as well as malformed text. Decoding must preserve bytes; charset enforcement needs column types and SQL mode. Mirroring WordPress's checks restores its expected validation behavior. Regression tests cover decoding, token ranges, version comments, and pattern conversion. CI expectations remove the `test_strip_invalid_text` cases fixed by decoding and the two invalid-query tests fixed by WordPress validation. ## Remaining limitations - Direct MySQL-on-SQLite writes can still accept invalid text under strict mode; full charset enforcement remains driver work. WordPress's checks retain their exemptions, including tables containing binary columns. - SQLite GLOB can still treat distinct invalid UTF-8 sequences as equal. The `LIKE BINARY` fix preserves pattern bytes but does not provide fully byte-correct matching. - Legacy multibyte charsets such as Big5 and GBK remain unsupported by the decoder.
batonac
changed the title
Brings in WordPress#496 (MySQL savepoint semantics), WordPress#500 (index prefix lengths in SHOW CREATE TABLE) and WordPress#505 (lexer edge cases and string handling). The one conflict is in the engine's transaction and savepoint handling: WordPress#496 reintroduced direct connection->query('BEGIN IMMEDIATE'/'COMMIT'/'ROLLBACK') plus its own $in_transaction and $savepoint_names tracking, while this branch routes transaction control through WP_SQLite_Connection_Interface so that remote backends can implement it themselves. Resolved by keeping upstream's semantics and this branch's dispatch: the new savepoint-name bookkeeping (MySQL replaces a savepoint on name reuse where SQLite shadows it, RELEASE and ROLLBACK TO drop the savepoints created after the named one, and an unknown name raises the new exception) is preserved, but the statements go through begin_transaction()/commit()/rollback()/savepoint()/ release_savepoint()/rollback_to_savepoint(). Upstream's engine-level $in_transaction polyfill is dropped as dead state, because inTransaction() on this branch already delegates to the connection. Verified: packages/mysql-on-sqlite 1080 tests, 1429272 assertions, no failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sync_column_key_info() assigned COLUMN_KEY and IS_NULLABLE together as a row
value fed by a correlated aggregate subquery. On SQLite that is safe: an
aggregate with no GROUP BY always yields exactly one row, so the CASE ELSE
branches always supply a value.
Turso does not always yield that row. When the row being updated sorts before a
row that does have a matching statistics entry, the subquery produces no row,
both columns are set to NULL, and the NOT NULL constraint on IS_NULLABLE fails:
NOT NULL constraint failed: _wp_sqlite_mysql_information_schema_columns.IS_NULLABLE
Reproduced against Turso 0.8.0-pre.7 with no PHP involved: two rows in a STRICT
table, only the later one indexed, and the same UPDATE. Whether it fails depends
purely on the sort order of the stored COLUMN_NAME values. WordPress hits it on
9 of the 12 core tables, because wp_users.comment_id-style columns sort before
the indexed primary key column -- so CREATE TABLE fails and wp_install() cannot
complete.
Assign the two columns separately and wrap each subquery in COALESCE(), falling
back to the values the ELSE branches would have produced for a column with no
index entries. Verified to produce identical results to the row-value form on
real SQLite, and to fix the failure on Turso.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
## Summary Make **schema reconstruction independent of `PDO::ATTR_STRINGIFY_FETCHES`**. Interpret SQLite's primary-key, nullability, and uniqueness flags as integers so reconstruction preserves auto-increment columns, `NOT NULL`, defaults, and unique indexes in either fetch mode. Add a regression test that creates an existing SQLite table before driver initialization and verifies its reconstructed column and index metadata with string fetching enabled and disabled. ## Why Reconstruction compared SQLite's numeric flags strictly against strings. Since connection initialization stopped forcing string results in WordPress#291, reconstruction can receive integers before WordPress enables string fetching. This could mark every column in an auto-increment table as auto-incrementing, causing a later `ALTER TABLE` to fail with "has more than one primary key." Related to WordPress#422. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved compatibility when SQLite returns column and index metadata as either strings or integers. * Information schema reconstruction now consistently handles numeric metadata values regardless of fetch settings or PHP version. * Schema and index details are reconstructed more reliably across different SQLite metadata formats. * **Tests** * Added coverage for schema reconstruction with stringified and native SQLite fetch values. * Verified reconstructed column metadata and index statistics across supported configurations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Release `3.0.2` Version bump and changelog update for release `3.0.2`. **Changelog draft:** * Fix schema reconstruction with native numeric results ([WordPress#506](WordPress#506)) * Fix lexer edge cases and string handling ([WordPress#505](WordPress#505)) * Preserve index prefix lengths and order in SHOW CREATE TABLE primary keys ([WordPress#500](WordPress#500)) * Align savepoint handling with MySQL semantics ([WordPress#496](WordPress#496)) **Full changelog:** WordPress/sqlite-database-integration@v3.0.1...release/v3.0.2 ## Next steps 1. **Review** the changes in this pull request. 2. **Push** any additional edits to this branch (`release/v3.0.2`). 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 --> ## Summary by CodeRabbit - **Bug Fixes** - Improved schema reconstruction when handling native numeric results. - Corrected lexer edge cases and string processing. - Fixed index prefix lengths and ordering in `SHOW CREATE TABLE` output for primary keys. - Improved savepoint handling to better match MySQL behavior. - **Release** - Updated the SQLite integration to version 3.0.2. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
batonac
changed the title
batonac
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Automated sync of
WordPress/sqlite-database-integration@trunkintod1-support.This pull request tracks every upstream commit that has not landed on
d1-supportyet.It is updated in place as upstream moves, and merges itself once the merge is clean and
every check is green. While it is open, it needs a person.
Upstream commits (2)
Branch shape:
merged. Maintained byupstream-sync.yml. Closing this pull request is not permanent — the next scheduled run reopens it. Disable the workflow to stop the sync.