search

doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.94k stars 2.52k forks source link

Doctrine tries to delete entities in incorrect order #9376

Closed jusephe closed 4 months ago

jusephe commented 2 years ago

Bug Report

Q A
BC Break no
Version 2.11.0

Summary

In some cases, Doctrine tries to delete entities in incorrect order and it fails on foreign keys exception. It also happens in 2.10.4

Current behavior

I have 6 entities with circular reference, and I tried to delete 2 of them. But UnitOfWork::commit() fails because of ForeignKeyConstraintViolationException.

The reason is that it first tries to delete the entity set as a foreign key in the second entity, which hasn't been deleted yet.

I investigated it quite a bit and the order of these operations is determined by $commitOrder in UnitOfWork::commit(). It is calculated from all 6 entities, but for the delete itself, the order of those 2 entities is not correct within those 6 entities. It should be sufficient to compute the order for deletions only with those 2 entities set to be deleted. I prepared a test with described behavior and a possible fix. See linked pull request.

How to reproduce

Run the test in a linked pull request with a DB platform supporting foreign key constraints. (i.e., not SQLite)

Expected behavior

UnitOfWork::commit() should delete entities. I prepared a possible fix in the linked pull request. I created a new method called getCommitOrderForDeletions() for computing the correct order of entities set to be deleted. This function is similar to getCommitOrder(), but it operates only with entities set to be deleted.

mpdude commented 1 year ago

The tests you provided in #9377 pass on #10547, so I assume that would fix this issue here as well.

Note your tests are much simpler than the case you describe in the OP, there is no circular dependency between 6 entities.

See https://github.com/doctrine/orm/pull/10547/commits/eee75bf226eb06c5711d1f8a159354388a3ee9ef

mpdude commented 1 year ago

I agree that regarding deletions, we need to find an ordering among the entities scheduled for deletion only. #10547 will do that.

The circular reference you mention in your opening comment is not part of the test case you provided in #9377.

Also without a circular reference, there might be problems with the delete commit order, but only when the order of entities of the same class is relevant.

The test case from #9377 passes on the current 2.16.x branch, so it seems not to be sufficient to show the bug (or the bug has already been fixed?).

/cc @greg0ire

mpdude commented 9 months ago

@jusephe do you still know how to reproduce the issue and do you think you could give #10913 a try?

jusephe commented 4 months ago

@mpdude Hi, sorry for late answer. The problem is fixed either by https://github.com/doctrine/orm/pull/10547 or https://github.com/doctrine/orm/pull/10913. Thank you very much! I am closing this.