Repository navigation
Fix temporary table lookup without stringified fetches and with a different case - #522
Conversation
…ferent case The temporary table lookup compared the fetched value to the string "1". PDO SQLite returns native types unless fetches are stringified, so on PHP 8.1 and newer, the lookup never found a temporary table for callers that don't set PDO::ATTR_STRINGIFY_FETCHES. Statements like ALTER TABLE, SHOW COLUMNS, or DESCRIBE then failed or used a standard table of the same name instead. The lookup also matched the table name case-sensitively, unlike the information schema tables, so a temporary table was not found when the statement used a differently cased name. Match the name case-insensitively and check the fetched value for presence rather than comparing it to a string. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
When translating a reference to an information schema table, the driver checked whether a temporary table with the same name existed, such as "columns". If it did, the query read the temporary table metadata instead of the standard one, so all standard tables disappeared from the result. With the case-insensitive temporary table lookup, any differently cased name triggers this too, e.g., a temporary table named "Columns". Temporary tables are not listed in the information schema, so always read the standard table metadata.
Test the differently cased name with a standard table of the same name, including DROP TABLE. Before the lookup fix, DROP TABLE removed the temporary table, but deleted the metadata of the standard table. Run the temporary table priority test with stringified fetches both enabled and disabled instead of keeping a partial copy of it.
|
Thanks for the contribution, @Nandan108! The fix looks good. I pushed two small improvements:
|
…532) ## Summary Fixes #451 together with #522. `ALTER TABLE`, `CREATE TABLE`, and `SHOW CREATE TABLE` compare fetched metadata to string values. PDO SQLite returns native types unless `PDO::ATTR_STRINGIFY_FETCHES` is set, so on PHP 8.1 and newer, these comparisons fail for callers that don't set that attribute. The WordPress plugin and the MySQL proxy set it, so they are unaffected, but the PDO-compatible API is: ```php $pdo = new WP_MySQL_On_SQLite( 'mysql-on-sqlite:dbname=wp', ... ); $pdo->query( 'CREATE TABLE u (id INT, email VARCHAR(50), UNIQUE KEY ue (email))' ); $pdo->query( "INSERT INTO u VALUES (1, 'a@example.com')" ); $pdo->query( "INSERT INTO u VALUES (2, 'a@example.com')" ); // Accepted: the unique key was created as a plain index. ``` The affected comparisons are the remaining ones listed in #451: - `NON_UNIQUE` is compared to `'0'` when generating the SQLite schema and when generating `SHOW CREATE TABLE` output, so every `UNIQUE KEY` becomes a plain index. That also breaks `INSERT ... ON DUPLICATE KEY UPDATE`, which then inserts duplicates. - `PRAGMA foreign_keys` is compared to `'1'` after `ALTER TABLE` recreates a table, so the foreign key check is skipped. #522 fixes the temporary table lookup, and #506 already fixed schema reconstruction. ## Fix The values are compared as integers. ## Tests `testUniqueKeysWithoutStringifiedFetches` disables stringified fetches, creates a table with a unique key, checks `SHOW CREATE TABLE`, recreates the table with `ALTER TABLE`, and checks that a duplicate row is rejected. It fails on `trunk`. Each of the two `NON_UNIQUE` comparisons fails it separately. The `PRAGMA foreign_keys` comparison has no test. `ALTER TABLE` runs inside the wrapper transaction, where `PRAGMA foreign_keys` is a no-op, so the skipped restore has no observable effect today. It is fixed for consistency. I also ran the whole suite with stringified fetches disabled in every test class. With this PR and #522, the only remaining failures are type differences in returned values (`1` vs `'1'`) and the two tests of the attribute itself. `packages/mysql-on-sqlite`: 1044 tests pass (PHP 8.3, SQLite 3.45.1); PHPCS is clean. ## Use of AI This change was prepared with Claude Code (Claude Opus 5.5). I reviewed the code, the tests, and this description, and I take responsibility for the change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Fixed unique database indexes being lost when tables are recreated after a schema change. Duplicate values in uniquely constrained fields are now correctly rejected. - Improved consistency when adding columns to tables: unique constraints are preserved whether database results are returned as strings or other value types. Database table definitions also continue to report unique indexes correctly in either case. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Jan Jakeš <jan@jakes.pro>
Summary
The internal check for whether a table is temporary (
temporary_table_exists()) has two defects. It is used byALTER TABLE,DROP TABLE,SHOW COLUMNS,SHOW CREATE TABLE,DESCRIBE,UPDATE, and others to decide whether to use the temporary or the standard information schema.It compares the fetched value to the string
'1'. PDO SQLite returns native types unlessPDO::ATTR_STRINGIFY_FETCHESis set, so on PHP 8.1 and newer, the check never finds a temporary table for callers that don't set that attribute. The WordPress plugin and the MySQL proxy set it, so they are unaffected, but the PDO-compatible API is:When a standard table of the same name exists, the statement silently applies to that table instead.
It matches the table name case-sensitively, unlike the information schema tables (
COLLATE NOCASE), so a temporary table is not found by a differently cased name:Fix
The lookup matches the name with
COLLATE NOCASEand checks the fetched value for presence (false !== fetchColumn()) rather than comparing it to a string. The translation tests filter this query out of the recorded SQLite statements by its text, so the filter is updated to match.Queries to the information schema no longer use this lookup with the name of the information schema table, such as
columns. Temporary tables are not listed in the information schema, but with a case-insensitive lookup, a temporary table namedColumnswould hide all standard tables frominformation_schema.columns.Tests
testTemporaryTableHasPriorityOverStandardTableruns with stringified fetches enabled and disabled.testTemporaryTableIsFoundByDifferentlyCasedNameuses a differently cased name inALTER TABLE,SHOW COLUMNS, andDROP TABLEwhile a standard table with the same name exists.testTemporaryTableNamedLikeInformationSchemaTablechecks that temporary tables named like information schema tables don't change the information schema results.All three fail on
trunk.Found while preparing a fix for
CREATE TEMPORARY TABLE IF NOT EXISTS, which will reuse this lookup.Part of #451.
Use of AI
This change was prepared with Claude Code (Claude Fable 5.1). I reviewed the code, the tests, and this description, and I take responsibility for the change.
🤖 Generated with Claude Code
Summary by CodeRabbit
ALTER TABLEandDROP TABLEcorrectly target temporary tables when names differ only by capitalization or overlap with a regular table.