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

Clean up SQL generation in MySQL platform #5071

Merged
merged 1 commit into from Dec 2, 2021

Conversation

morozov
Copy link
Member

@morozov morozov commented Dec 2, 2021

Q A
Type improvement
BC Break no

As identified in #4978 (comment), the query generated by getListTableForeignKeysSQL() uses a DISTINCT clause which requires selecting the columns used in the ORDER BY clause.

What has been changed:

  1. The DISTINCT clause has been removed since it's not necessary.
  2. The WHERE k.REFERENCED_COLUMN_NAME IS NOT NULL condition has been removed since it's not necessary.
  3. The WHERE c.constraint_schema = $databaseNameSql condition has been replaced with an analogous ON condition.
  4. The original (upper) column case is used with the redundant quotes removed.

Additionally, the following has been done in multiple queries:

  1. The logic of quoting the database name or defaulting to DATABASE() has been extracted into a private method.
  2. All occurrences like $string = quote($string) have been replaced with inlining with the concatenation to avoid potential double quoting.

The last two changes likely would have happened in #4882 but it's better to do the cleanup separately.

@morozov morozov marked this pull request as ready for review December 2, 2021 03:03
@morozov morozov added the MySQL label Dec 2, 2021
@morozov morozov added this to the 3.3.0 milestone Dec 2, 2021
@morozov morozov merged commit b46c1aa into doctrine:3.3.x Dec 2, 2021
@morozov morozov deleted the mysql-platform-sql-cleanup branch January 1, 2022 00:20
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants