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

Fix dropping the PK on a PostgreSQL table with quoted name #6599

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

morozov
Copy link
Member

@morozov morozov commented Nov 14, 2024

Q A
Type bug

The issue was discovered during the work on #6589. If a table name passed to PostgreSQLPlatform#getDropIndexSQL() is quoted, it will generate invalid SQL (see the test).

The fix is a quite ugly as the code parses the provided string. This is inherent to the current platform and schema management API design where string $name can contain virtually anything (quoted or unquoted, qualified or unqualified name) and is meant to be used to in concatenation with other SQL fragments, not parsed. We will address this design issue later by making object names value objects.

@greg0ire
Copy link
Member

greg0ire commented Nov 14, 2024

Does this not affect v3? I'm on my phone and cannot check easily

@morozov
Copy link
Member Author

morozov commented Nov 14, 2024

It does, I tried to back-port but decided not to:

  1. It won't cleanly rebase because of the differences in the method signature.
  2. Given that the default branch in this repo is 4.2.x, I'm not sure what the support status of 3.x is.

My view is that if anyone is affected by this issue on version 3.x, they can back-port the fix.

@morozov morozov merged commit 727bd6b into doctrine:4.2.x Nov 14, 2024
88 of 89 checks passed
@morozov morozov deleted the postgresql-quoted-drop-pk branch November 14, 2024 20:08
@greg0ire
Copy link
Member

Regarding 2. to figure if a branch is maintained, I refer to .doctrine-project.json

We usually merge things up but having people backport stuff they need makes sense, it makes the maintenance work easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants