dbal icon indicating copy to clipboard operation
dbal copied to clipboard

Fixed missing schema on MySQL table names

Open jochen-jung opened this issue 4 months ago • 7 comments

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

jochen-jung avatar Sep 02 '25 09:09 jochen-jung

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?

derrabus avatar Sep 04 '25 21:09 derrabus

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.

jochen-jung avatar Sep 05 '25 05:09 jochen-jung

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.

jochen-jung avatar Sep 09 '25 06:09 jochen-jung

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?

morozov avatar Sep 10 '25 02:09 morozov

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.

jochen-jung avatar Sep 10 '25 07:09 jochen-jung

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

morozov avatar Sep 10 '25 13:09 morozov

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.

jochen-jung avatar Sep 10 '25 13:09 jochen-jung