Skip to content

#185 - Version20261005120000: disable foreign_key_checks around the NOT NULL change - #189

Merged
njoubert-cleverage merged 2 commits into
mainfrom
185
Oct 20, 2026
Merged

njoubert-cleverage merged 2 commits into
mainfrom
185

Conversation

@xaviermarchegay

@xaviermarchegay xaviermarchegay commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Description

Fixes #185: on MySQL, Version20261005120000::up() disables foreign_key_checks around the change of log_record.process_execution_id to NOT NULL (MariaDB shares the same code, it was not affected). The foreign key FK_8ECECC333DAC0075 is kept as is (ON DELETE CASCADE).

Without a strict SQL mode, MySQL makes the column NOT NULL with ALGORITHM=COPY, which is refused on a foreign key column while foreign_key_checks is enabled: the migration failed with "1832 Cannot change column 'process_execution_id': used in a foreign key constraint 'FK_8ECECC333DAC0075'". No integrity check is really skipped: the MODIFY does not change any value, and the NULL rows are deleted just before. PostgreSQL and down() are unchanged.

Dropping and adding the foreign key back (first version of this PR) was replaced after the review: adding a foreign key always copies the whole log_record table and blocks writes, including in strict mode, and a failure halfway could not be retried.

Why a released migration is changed

Version20261005120000 is part of v3.1, v3.1.1 and v3.2:

  • A new migration would not help: it would run after the failing one, which blocks all the following migrations (Version20261008120000, Version20261013120000, Version20261014120000).
  • Installations where the migration already succeeded are not affected: it is recorded and never run again, and the resulting schema is the same.
  • Installations where it failed can simply run it again after the update: the failed MODIFY changed nothing, and the DELETE that was already committed is harmless to run again. If the workaround of Migrations: Version20261005120000 fails on MySQL without a strict SQL mode (error 1832 on the log_record foreign key) #185 was applied (doctrine:migrations:version ... --add), there is nothing to do.

Changes

  • Migration: SET foreign_key_checks = 0/1 around the MODIFY.
  • CI: the migrations job (which runs migrate, schema:validate, migrate first, then migrate again on MySQL 8.4, MariaDB 11.4 and PostgreSQL 17, with DBAL 3 and 4, in the default strict SQL mode) gets a MySQL 8.4 + DBAL 4 entry without a strict SQL mode (SET GLOBAL sql_mode = ''), which fails with the released migration (1832).
  • CHANGELOG.

Checked on a MySQL 8.4 container with sql_mode='': the column is NOT NULL, FK_8ECECC333DAC0075 keeps ON DELETE CASCADE. make quality passes.

Requirements

  • Documentation updates
    • Reference
    • Changelog
  • Tests (CI: migrations on MySQL without a strict SQL mode)

Breaking changes

None: same resulting schema; the migration failed in this configuration.

🤖 Generated with Claude Code

…reign key around the NOT NULL change

Without a strict SQL mode, MySQL makes the column NOT NULL with
ALGORITHM=COPY, which is refused on a foreign key column (error 1832).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@xaviermarchegay xaviermarchegay added the bug Something isn't working label Oct 20, 2026

@njoubert-cleverage njoubert-cleverage left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix works, but I'd change the implementation before merging. It does fix error 1832 on MySQL without a strict SQL mode. However, dropping and adding the foreign key back makes MySQL and MariaDB copy the whole log_record table, blocking writes while it runs. Every MySQL / MariaDB installation that has not run the migration yet pays for this, including the strict-mode ones (the default and the majority), which were never affected by the bug. Wrapping only the MODIFY with SET foreign_key_checks = 0/1 fixes the bug without that cost (see 1. and the inline suggestion).

Checked on real servers

Throwaway containers. log_record holds 1,048,576 rows, with FK_8ECECC333DAC0075 ON DELETE CASCADE as created by Version20231006111525.

sql_mode='' before sql_mode='' PR strict before strict PR foreign_key_checks=0 (non-strict / strict)
MySQL 8.4.2 ❌ 1832 ✅ 33.8 s ✅ 9.7 s ✅ 39.4 s ✅ 15.5 s / 9.6 s
MySQL 8.0.34 ❌ 1832 ✅ 63.9 s ✅ 48.0 s ✅ 78.3 s ✅ 30.2 s / 47.5 s
MariaDB 11.4.13 ✅ 5.4 s ✅ 16.3 s ✅ ✅ ✅ 5.9 s / ✅
MariaDB 10.6.28 (200k rows) ✅ ✅ ✅ ✅ ✅ / ✅

Every successful run gives the same result: the column is NOT NULL and FK_8ECECC333DAC0075 keeps ON DELETE CASCADE. The MySQL 8.0 timings are noisy (the machine was busy).

  • ADD CONSTRAINT ..., ALGORITHM=INPLACE is refused on both MySQL and MariaDB: "ALGORITHM=INPLACE is not supported. Reason: Adding foreign keys needs foreign_key_checks=OFF. Try ALGORITHM=COPY". The foreign key added back by the PR always means a full table copy.
  • In strict mode, the original statement succeeds with ALGORITHM=INPLACE, LOCK=NONE. The PR replaces this in-place change, which does not block writes, with a copy that does.
  • A single ALTER TABLE ... DROP FOREIGN KEY fk, MODIFY ..., ADD CONSTRAINT fk ... is not an option: it fails with 1826 Duplicate foreign key constraint name on MySQL, and 1005 / errno 121 on MariaDB.
  • MariaDB is not affected by #185: the original statement succeeds with sql_mode='' on 10.6 and 11.4.

1. Performance and locking: prefer foreign_key_checks = 0 (major)

log_record is the biggest table of the bundle. With the alternative, the foreign key is never dropped and its definition (ON DELETE CASCADE) does not need to be repeated in this migration. No real integrity check is skipped: the MODIFY does not change any value, and the NULL rows are deleted just before. See the inline suggestion.

If the MODIFY fails, the session stays with foreign_key_checks = 0. The migration process stops anyway in that case, so this is acceptable.

2. A failure halfway cannot be retried (minor, solved by 1.)

MySQL commits each DDL statement implicitly, so the three statements are not atomic. If adding the foreign key back fails (e.g. an orphan row inserted once with foreign_key_checks=0, or a lock wait timeout during the copy), the foreign key stays dropped. Running the migration again then fails on its first statement:

1st run: ERROR 1452 (23000): Cannot add or update a child row: a foreign key constraint fails
2nd run: ERROR 1091 (42000): Can't DROP 'FK_8ECECC333DAC0075'; check that column/key exists

Reproduced on MySQL 8.0 / 8.4 and MariaDB 10.6 / 11.4.

3. Changing a released migration (to state explicitly)

Version20261005120000 is part of v3.1, v3.1.1 and v3.2. We normally never change a released migration, but this exception looks justified to me. I'd state the reasons in the PR description:

  • A new migration would not help: it would run after the failing one, which blocks all the following migrations (Version20261008120000, Version20261013120000, Version20261014120000).
  • Installations where the migration already succeeded are not affected: it is recorded and never run again, and the resulting schema is the same.
  • Installations where it failed can simply run it again after the update. The failed MODIFY changed nothing, so the foreign key is still there, and the DELETE that was already committed is harmless to run again.

4. CHANGELOG (minor)

  • "MySQL / MariaDB": only MySQL failed. I'd say e.g. "MySQL (MariaDB shares the same code)", so that readers don't think MariaDB failed too.
  • Add a line for whoever hit the bug: run the migrations again after the update. If the workaround of #185 was applied (doctrine:migrations:version ... --add), there is nothing to do.
  • If 1. is applied, "dropped and created again" no longer describes the change.

5. PR description: the CI does run the migrations on MySQL (minor)

"the migrations are not run on MySQL by the tests" is not accurate. The migrations job in .github/workflows/test.yml runs migrate, schema:validate, migrate first, then migrate again, on MySQL 8.4, MariaDB 11.4 and PostgreSQL 17, with DBAL 3 and 4. It does so with the default (strict) SQL mode, though, so it cannot catch #185.

A cheap addition: a MySQL matrix entry with a non-strict SQL mode, set before Run the migrations:

- name: Non-strict SQL mode
  if: matrix.sql-mode == 'non-strict'
  run: mysql -h127.0.0.1 -uroot -proot -e "SET GLOBAL sql_mode = ''"

It fails with the released migration (1832) and passes with this fix. It would also cover any future migration that makes a column NOT NULL.

6. Nothing to change

  • down(): making a column nullable is done in place whatever the SQL mode, so leaving it as is (rather than aligning it "for consistency", as suggested in the issue) is the right call.
  • PostgreSQL: unchanged, fine.
  • FK_8ECECC333DAC0075 is a deterministic Doctrine name. It is the same even for tables created outside the migrations (the hasTable() guards of Version20231006111525, doctrine:schema:update).
  • No new DBAL API is used, so there is nothing to check for DBAL 3 / 4.
  • Commit and title convention, bug label, and CI (25/25 green) are fine.

Summary

# Severity Request
1 Major Replace dropping and adding the foreign key with SET foreign_key_checks = 0/1 around the MODIFY (no table copy, no blocked writes, no slowdown in strict mode)
2 Minor Solved by 1. (a failure halfway cannot be retried)
3 To state Explain in the PR why a released migration (v3.1+) is changed
4 Minor CHANGELOG: only MySQL failed; say what to do after a failure or the workaround
5 Minor Fix the CI sentence in the PR description; consider a non-strict MySQL variant in the migrations job

Comment on lines +45 to +49
// Without a strict SQL mode, MySQL makes the column NOT NULL with ALGORITHM=COPY, which is refused on a
// foreign key column (error 1832): the foreign key is dropped and created again around the change
$this->addSql('ALTER TABLE log_record DROP FOREIGN KEY FK_8ECECC333DAC0075');
$this->addSql('ALTER TABLE log_record MODIFY process_execution_id INT NOT NULL');
$this->addSql('ALTER TABLE log_record ADD CONSTRAINT FK_8ECECC333DAC0075 FOREIGN KEY (process_execution_id) REFERENCES process_execution (id) ON DELETE CASCADE');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (see 1. in the review): keep the foreign key and only disable its check around the MODIFY. Same result (checked on MySQL 8.0 / 8.4, MariaDB 10.6 / 11.4, strict and non-strict), no full table copy when adding the foreign key back, no slowdown in strict mode, and nothing is left half-done if a statement fails.

Suggested change
// Without a strict SQL mode, MySQL makes the column NOT NULL with ALGORITHM=COPY, which is refused on a
// foreign key column (error 1832): the foreign key is dropped and created again around the change
$this->addSql('ALTER TABLE log_record DROP FOREIGN KEY FK_8ECECC333DAC0075');
$this->addSql('ALTER TABLE log_record MODIFY process_execution_id INT NOT NULL');
$this->addSql('ALTER TABLE log_record ADD CONSTRAINT FK_8ECECC333DAC0075 FOREIGN KEY (process_execution_id) REFERENCES process_execution (id) ON DELETE CASCADE');
// Without a strict SQL mode, MySQL makes the column NOT NULL with ALGORITHM=COPY, which is refused on a
// foreign key column while foreign_key_checks is enabled (error 1832). The log records without process
// execution are deleted above, the foreign key is kept as is
$this->addSql('SET foreign_key_checks = 0');
$this->addSql('ALTER TABLE log_record MODIFY process_execution_id INT NOT NULL');
$this->addSql('SET foreign_key_checks = 1');

…OT NULL change instead of dropping the foreign key

Dropping and adding the foreign key back copies the whole log_record table
and blocks writes, also in strict mode. foreign_key_checks = 0 keeps the
foreign key and its in-place change. CI: run the migrations on MySQL
without a strict SQL mode too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@xaviermarchegay xaviermarchegay changed the title #185 - Version20261005120000: drop and create again the log_record foreign key around the NOT NULL change #185 - Version20261005120000: disable foreign_key_checks around the NOT NULL change Oct 20, 2026
@njoubert-cleverage
njoubert-cleverage merged commit 4259e8b into main Oct 20, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrations: Version20261005120000 fails on MySQL without a strict SQL mode (error 1832 on the log_record foreign key)

2 participants