Preserve index prefix lengths and order in SHOW CREATE TABLE primary keys - #500
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe table-definition generator now uses shared formatting for primary and secondary index columns. The formatter preserves column names, prefix lengths, and descending order. Metadata tests cover single-column and composite primary keys. ChangesMySQL-on-SQLite metadata
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change preserves primary-key prefix lengths and descending order in generated table definitions, preventing invalid exports while leaving other index formatting unchanged. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
9e45b6e to
ae52afd
Compare
|
Thanks for the fix, looks good! |
## Release `3.0.2` Version bump and changelog update for release `3.0.2`. **Changelog draft:** * Fix schema reconstruction with native numeric results ([#506](#506)) * Fix lexer edge cases and string handling ([#505](#505)) * Preserve index prefix lengths and order in SHOW CREATE TABLE primary keys ([#500](#500)) * Align savepoint handling with MySQL semantics ([#496](#496)) **Full changelog:** 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 -->
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>
Summary
SHOW CREATE TABLEreconstructs thePRIMARY KEYclause 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 withPRIMARY KEY (session_id(100))on aMEDIUMTEXTcolumn came back asPRIMARY KEY (session_id).The practical impact: exports built on top of
SHOW CREATE TABLE(for examplewp sqlite export, which WordPress Studio uses for its push feature) produce a dump that MySQL rejects withERROR 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 #501
Fix
The column formatting closure that already handled
SUB_PARTandDESCfor regular keys is hoisted and shared by thePRIMARY KEYbranch, so both paths emit identical column definitions.Testing
PRIMARY KEY (session_id(100))onMEDIUMTEXTround-trips throughSHOW CREATE TABLEwith its prefix, a composite keyPRIMARY KEY (a, b(50))keeps the prefix only where defined, andPRIMARY KEY (a DESC, b)keeps its descending key part. The test fails on the previous code.SHOW CREATE TABLEemitsPRIMARY KEY (session_id(100))andPRIMARY KEY (aDESC,b)for the same tables, matching the fixed output; the previous output fails to import there withERROR 1170, the fixed output imports cleanly.mysql-on-sqliteunit suite: 875 tests, 1428750 assertions, no failures (the same 17 skipped and 2 incomplete as on trunk, PHP 8.5).composer run check-csis clean on both changed files.Summary by CodeRabbit
SHOW CREATE TABLEoutput to preserve primary-key index prefix lengths for single-column and composite keys.