Skip to content

Fix schema reconstruction with native numeric results - #506

Open
JanJakes wants to merge 1 commit into
trunkfrom
reconstruction-fetch-flags
Open

Fix schema reconstruction with native numeric results#506
JanJakes wants to merge 1 commit into
trunkfrom
reconstruction-fetch-flags

Conversation

@JanJakes

@JanJakes JanJakes commented Sep 9, 2026

Copy link
Copy Markdown
Member

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 #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 #422.

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.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: eade14f6-c3af-409d-99e1-ec3fd342dd5f

📥 Commits

Reviewing files that changed from the base of the PR and between 743439f and 71c946c.

📒 Files selected for processing (1)
  • packages/mysql-on-sqlite/tests/WP_SQLite_Information_Schema_Reconstructor_Tests.php

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The information schema reconstructor now casts SQLite metadata flags to integers before comparison. Parameterized tests verify reconstruction with native and stringified PDO fetch values.

Changes

SQLite metadata normalization

Layer / File(s) Summary
Normalize metadata flag comparisons
packages/mysql-on-sqlite/src/sqlite/class-wp-sqlite-information-schema-reconstructor.php
The reconstructor casts notnull, pk, and unique metadata values to integers before comparison.
Validate stringified fetches
packages/mysql-on-sqlite/tests/WP_SQLite_Information_Schema_Reconstructor_Tests.php
Parameterized tests cover native and stringified PDO fetch values and verify reconstructed column and index metadata.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 71c94

Schema reconstruction now preserves SQLite column and index metadata regardless of PDO numeric fetch representation. The change includes coverage for both stringified and native numeric results, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making schema reconstruction work with native numeric SQLite results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch reconstruction-fetch-flags

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JanJakes
JanJakes force-pushed the reconstruction-fetch-flags branch 2 times, most recently from 6068950 to 743439f Compare September 9, 2026 14:18
@JanJakes
JanJakes marked this pull request as ready for review September 9, 2026 14:19
Normalize SQLite’s numeric schema flags before strict comparisons.
Cover reconstruction with string fetching enabled and disabled,
including column defaults, nullability, and indexes.

#422
@JanJakes
JanJakes force-pushed the reconstruction-fetch-flags branch from 743439f to 71c946c Compare September 9, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant