Fixed missing schema on MySQL table names
| Q | A |
|---|---|
| Type | bug |
| Fixed issues | (https://github.com/doctrine/migrations/issues/1460) |
Summary
The schema was missing on the table name for MySQL.
Because of that "bin/console doctrine:migrations:diff" did not generate correct diffs and always detected the whole table as changed.
See: https://github.com/doctrine/migrations/issues/1460
Looks like your change broke a lot of existing tests and you didn't add any new tests that would reproduce the issue that you're trying to solve. Are you planning to continue your work?
Hi, I was hoping for some feedback whether the change is fine this way in general, as I do not know the doctrine code well enough.
But I can try to resolve the unit tests first next Monday, if you prefer.
I got the tests fixed now.
As I prepend the schema to the table name only for MySQL (I don't know if that makes sense for other engines as well), I added some ifs to the tests, so in case of MySQL the schema is prepended in the test as well.
For the tests/Functional/Schema/SchemaManagerFunctionalTestCase.php I overwrite the problematic tests now inside tests/Functional/Schema/MySQLSchemaManagerTest.php with a version that prepends the schema.
As for the change in src, another option would be instead of doing the concat on the database it could be done in the _getPortableTableDefinition() method instead, if that is considered cleaner. But I decided against that, as the method is marked as deprecated.
The DBAL doesn't support schemas on MySQL since it's a synonym for databases. Furthermore, on those platforms where the DBAL supports schemas (PostgreSQL, SQL Server), the current schema name is omitted from the introspected name. Thus, if the model defines the table name as users, it's created as public.users but then is introspected again as users.
What makes you think that this change needs to be made in the DBAL?
We have a project with MySql that has tables on different schemata (or tables in different tables, if schema and table is the same thing).
schemaA.table1 schemaA.table2 schemaB.table1 ...
On the entities we have: ORM\Table(name: 'table1', schema: 'schemaA')
And that is what is not working. For if I now generate the migration using bin/console doctrine:migrations:diff it detects the whole entity as new and not only the field that I modified. Same for indexes, foreign keys, ...
Also using ORM\Table(name: 'schemaA.table1') leads to the same error.
The patch I provided fixes this. It then works with ORM\Table(name: 'table1', schema: 'schemaA')
and generates a correct migration, that only contains the one change I did in the entity.
I believe that this is only an issue when a project uses more than one schema. If everything is inside the default schema, then it is no issue and it is enough to say 'table1', but in our case where there are different schemas inside a single project the schema name needs to be prepended to the table name.
We have a project with MySql that has tables on different schemata (or tables in different tables, if schema and table is the same thing).
schemaA.table1 schemaA.table2 schemaB.table1
This is not supported. The DBAL can introspect only a single MySQL database (which is a synonym for schema in MySQL) at a time. See the code: https://github.com/doctrine/dbal/blob/a1f4d020e503edb7c65a04a681b12bcf3fbd0ea3/src/Schema/MySQLSchemaManager.php#L345-L351
Then why not support it?
I understand that you can not support it the way you support schemas on other engines.
But to me it looks like all that is needed, is to add the schema to the tables names like in this pull request. Then it could be supported.
At least in the migrations:diff it works then. I don't have enough overview over the doctrine/dbal to know where else stuff would be affected.