Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MySQL: Determining existing composite foreign keys does not take column positions (ORDINAL_POSITION) into account (missing ORDER BY) #4971

Closed
AndreasA opened this issue Nov 9, 2021 · 4 comments · Fixed by #4978

Comments

@AndreasA
Copy link

AndreasA commented Nov 9, 2021

Bug Report

When using a foreign key with multiple join columns, the method \Doctrine\DBAL\Platforms\MySqlPlatform::getListTableForeignKeysSQL does not order by ORDINAL_POSITION in its SQL. See:

public function getListTableForeignKeysSQL($table, $database = null)

This can result in the order of the columns not being the same as expected.

E.g. on a MySQL 8.0 installation it seems to be ordered alphabetically by column name and 5.7 it seems to be by ordinal position which results in potential schema differences with 8.0

It might also be something else and not version related.

I think it should be enough to add

ORDER BY k.`ORDINAL_POSITION` ASC

Not 100% sure if this is the correct field but I think it is. It probably also has to be added to the select for the ORDER BY to be usable in this scenario.

With it, 8.0 and 5.7 both return the same order for the referenced columns.

For other similar statements the columns are already ordered, e.g.

public function getListTableIndexesSQL($table, $database = null)

or
public function getListTableColumnsSQL($table, $database = null)

Q A
BC Break no
Version 2.13.4 (but probably also 3.1.3)
@AndreasA AndreasA changed the title MySQL: Determining existing composite foreign keys does not take column positions (ORDINAL_POSITION) into account (missing ORDER BY) MySQL: Determining existing composite foreign keys does not take column positions (ORDINAL_POSITION) into account (missing ORDER BY) Nov 9, 2021
@morozov morozov added the Bug label Nov 9, 2021
@morozov
Copy link
Member

morozov commented Nov 9, 2021

Unless the missing ORDER BY impacts the behavior from the API consumer perspective, I don't believe it's a bug. It's pretty much possible that the rows are ordered in the column order in the INFORMATION_SCHEMA.KEY_COLUMN_USAGE table.

E.g. on a MySQL 8.0 installation it seems to be ordered alphabetically by column name and 5.7 it seems to be by ordinal position which results in potential schema differences with 8.0

If it's a real issue, it should be reproducible with an integration test.

@AndreasA please feel free to send a patch though.

@AndreasA
Copy link
Author

AndreasA commented Nov 9, 2021

@morozov It is in so far reproducible that if I change the order of the join columns it tries to drop on a schema update, thene re-add the key again, etc. as the generated index name is different from the expected one due to different column order.

If I find the time I will create an integration test plus fix.

@AndreasA
Copy link
Author

AndreasA commented Nov 10, 2021

@morozov I have now created a PR #4978

If I remove this line https://github.com/doctrine/dbal/pull/4978/files#diff-f2c2d9a23d4985a19b3d2df3e3ff932fa5a506eae05cacbf084ff497362bf12aR199

then the created test https://github.com/doctrine/dbal/pull/4978/files#diff-6eb874abe74292a78033e7a9b66317b16aaed0ab8eb6ce1e36dd1801d8fe897dR547

fails in my MySQL 8.0.26 installation and it definitely shouldn't fail because doctrine DBAL thinks that the table it created itself differs from the created one but it doesn't.

I would actually also like to try that the test also fails in the CI with that adjustment first but the pipelines do not start for me there because maybe it is also related to some my.cnf configuration though I don't have anything special in there.

@morozov morozov added the Bug label Nov 11, 2021
@morozov morozov linked a pull request Nov 14, 2021 that will close this issue
@morozov morozov added this to the 3.1.4 milestone Nov 14, 2021
@morozov morozov closed this as completed Nov 14, 2021
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants