TASK: Use migrations in pipeline and fixed migrations for MariaDB 12.x#3571
TASK: Use migrations in pipeline and fixed migrations for MariaDB 12.x#3571dlubitz wants to merge 1 commit into
Conversation
bwaidelich
left a comment
bwaidelich
left a comment
There was a problem hiding this comment.
Thanks for the description.
So I guess we accept the fact that a down migration would not re-add the FK constraints, right?
|
Yes and no. If you only go down one version, this will not be an issue, as the table has not been renamed in meanwhile. But if you would go down many versions until the renaming, this might have an effect. But TBH, this shouldn't be a valid use case. AND target should be anyways to get rid of these old old migrations by compression. |
bwaidelich
left a comment
bwaidelich
left a comment
There was a problem hiding this comment.
I agree, thanks!
+1 by reading
| - name: Run migrations to test if they work | ||
| run: FLOW_CONTEXT=Testing ./flow doctrine:migrate |
There was a problem hiding this comment.
+1 def makes sense to include that ... even though the functional test will use sqlite and thus not use the prepared database ...
|
|
||
| if ($this->isPartyPackageInstalled()) { | ||
| $this->addSql("ALTER TABLE typo3_flow3_security_account DROP FOREIGN KEY typo3_flow3_security_account_ibfk_1"); | ||
| foreach ($this->sm->listTableForeignKeys('typo3_flow3_security_account') as $foreignKey) { |
There was a problem hiding this comment.
ill find these migrations super hard to read now ... i think we discussed multiple times to have a merged migration state with Neos 10.0 neos/neos-development-collection#5656 can you say if we also need to use these kind of checks when the migrations are compacted or is this then an issue of the past?
TBH i dont understand why we not just define the names of the foreign keys during the creation and thus never have the problem of anything being "random" or autogenerated...
There was a problem hiding this comment.
This question is to late. As we already have this situation now and we need to handle that.
There was a problem hiding this comment.
Fully understood. I meant i am more willing to accept this state if we know its definitely just $temporary
There was a problem hiding this comment.
but i dont understand the problem well enough still because
why we not just define the names of the foreign keys during the creation and thus never have the problem of anything being "random" or autogenerated...
3 participants
What I did
Some old Flow migrations dropped foreign keys by their hardcoded auto-generated names (e.g.
..._ibfk_1). These names are no longer reliable: on MariaDB 12+, renaming a table no longer renames its auto-generated foreign key/index names along with it, so the constraint names the migrations expect may not exist and the migrations fail.Instead of relying on hardcoded constraint names, the affected migrations now look up the actual foreign keys on the table via the schema manager and drop the one defined on the relevant column. This works regardless of what the constraint is actually named, making the migrations compatible with MySQL, MariaDB < 12 and MariaDB ≥ 12 alike.
Why the CI change
Until now this breakage went unnoticed because the pipeline never executed the Doctrine migrations. The build workflow now runs
./flow doctrine:migratebefore the functional tests, so the full migration chain is verified on every build.