Stop migrate re-declaring keys and chasing integer widths

[alexstandiford]

Why

syncColumns could not complete against MySQL 8 at all. Any project calling it got Multiple primary key defined and the migration aborted, which means no column could be added to any existing table on a deployed environment.

Found while trying to add a column in Navigator. I had started designing around the failure before realising the tool was the thing that needed fixing.

The three defects

1. The key clause was re-declared on MODIFY. A primary key column whose type had drifted produced:

MODIFY COLUMN `id` BIGINT AUTO_INCREMENT PRIMARY KEY

The key already exists, so the server rejects the whole statement. Keys are owned by the table's index definitions, not by a column modification. The clause is now stripped on MODIFY and kept on ADD, where there is no existing key to collide with. AUTO_INCREMENT stays in both, since that genuinely is a column property.

2. Integer display widths were treated as differences. MySQL discards them as of 8.0.17, so a schema declaring INT(11) reads back as plain int. Every integer column therefore looked permanently out of date. Against one real schema, migrate wanted to rewrite 43 tables that needed nothing. After the fix: 5, all genuine drift.

3. The comparison used str_contains. bigint contains int, so a real widening from int to BIGINT was read as already satisfied and silently skipped. It now compares normalized types for equality, which catches the widening while still ignoring the discarded width.

Defects 2 and 3 are the same mistake in both directions: a substring test standing in for a type comparison.

Evidence

Nine new unit tests, each pinning one behaviour. Four of them failed before this change:

✔ Integer display width is not a difference
✔ Bigint display width is not a difference
✔ Widening an int to bigint is still detected
✔ Modify does not re declare the primary key
✔ Adding a new primary key column keeps the key clause
✔ A genuinely new column is added
✔ A removed column is dropped
✔ A varchar length change is detected
✔ A table in sync produces no query

Full suite: OK (12 tests, 31 assertions).

Verified against a live MySQL 8 database rather than only in mocks:

• migrate completes where it previously aborted.
• Two id columns actually widened from int to bigint.
• SHOW KEYS confirms the primary keys survived.
• A second run is a clean no-op, and the pending-change count drops to zero.

Note on the last test

test_a_table_in_sync_produces_no_query is the one that matters most for anyone operating this. Before the change, a fully in-sync table still emitted an ALTER, so migrate was never safe to run casually against production.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e43468b030

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve newly declared keys on existing columns

When an existing non-key column is changed to a primary or unique key while its type also changes, always passing false strips the newly requested key clause. For example, migrating an existing INT id to the BIGINT PRIMARY KEY produced by PrimaryKeyFactory changes the type but never creates the primary key; later runs see the matching type and never retry the constraint. Only suppress a key that is confirmed to exist, or emit the key change separately.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Normalize type modifiers before exact comparison

For schema columns with modifiers such as UNSIGNED, these operands use different representations: COLUMN_TYPE is bigint unsigned, while declaredType() returns only bigint because UNSIGNED lives in the column attributes. The exact comparison therefore emits the same MODIFY COLUMN ... BIGINT UNSIGNED on every synchronization even though the database is already correct; server-expanded defaults such as DECIMAL becoming decimal(10,0) have the same problem. Compare parsed base types and modifiers/defaults, or construct both normalized values from equivalent inputs.

Useful? React with 👍 / 👎.