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

Identifier type is not set when many2many relations are deleted #8401

Merged
merged 3 commits into from
Feb 16, 2021

Conversation

yoshz
Copy link
Contributor

@yoshz yoshz commented Dec 19, 2020

I was trying out the new Symfony Ulid doctrine type together with Postgres and found a small bug.
For this use case the Ulid is formatted as UUID so it can be stored using the native GUID type.
In Doctrine this works fine until you want to delete an entity that has many2many relations causing error
SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for type uuid

This PR makes sure that the identifier types are set on the delete queries so the Ulid is mapped correctly.

@beberlei
Copy link
Member

This looks good, however it also is confusing this wasn't found before.

@SenseException
Copy link
Member

This PR also needs tests for this fix.

@yoshz yoshz changed the base branch from master to 2.8.x February 2, 2021 09:16
@yoshz
Copy link
Contributor Author

yoshz commented Feb 2, 2021

Added the unittest and tried to fix the failing checks.
The coding standards check is conflicting with support for < PHP 7.4 so can't fix that.

@SenseException
Copy link
Member

You're referring to property types, right? You can extend phpcs.xml.dist for those checks that are only supported by PHP 7.4.

@yoshz
Copy link
Contributor Author

yoshz commented Feb 3, 2021

Phpcs is complaining about native return type hint, native property type hint and more that doesn't make sense.
I see that on other PR's the phpcs check also fails, so I am going to ignore that.

Hopefully the unittest covers the change enough so this bugfix can be merged.

@beberlei
Copy link
Member

beberlei commented Feb 5, 2021

@yoshz we merged a huge PR cleaning up code style into 2.8 yesterday, so there are now conflics. However after merging 2.8.x again you can now run php vendor/bin/phpcbf for autofixes safely.

@beberlei beberlei added this to the 2.8.2 milestone Feb 5, 2021
@beberlei beberlei added Bug and removed Missing Tests labels Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants