Compute particular commit order for deletions#9377
Compute particular commit order for deletions#9377jusephe wants to merge 1 commit intodoctrine:2.13.xfrom
Conversation
c47ddcc to
dbd630e
Compare
|
I just pushed new commit to fix "Static Analysis with Psalm". |
dbd630e to
c58dea4
Compare
c58dea4 to
2d301aa
Compare
|
It looks like your test does not cover your fix, because when I undo your changes in UnitOfWork, the test doesn't fail. ORM seems to be able to handle the test case. |
|
@SenseException Have you tried with a DB platform supporting foreign key constraints. (i.e., not SQLite)? I tried it again and it still fails... |
2d301aa to
ff5ce3e
Compare
|
@SenseException Can I help you somehow with testing the issue? I rebased my branch onto current version and the test still fails if I undo my changes. The test has to be run with a DB platform supporting foreign key constraints. (i.e., NOT SQLite). |
|
Sorry for the late answer and for forgetting about this PR. The commit order calculation isn't an easy topic and I'll have to take more time to review it. I also want to prevent possible impacts on performance. |
|
Please change the base branch to 2.12.x and rebase. |
ff5ce3e to
43f1e3e
Compare
|
@SenseException I rebased my branch onto 2.12.x. The test still fails if I undo my changes. (The test has to be run with a DB platform supporting foreign key constraints (i.e., NOT SQLite). |
SenseException
left a comment
There was a problem hiding this comment.
@doctrine/doctrinecore
43f1e3e to
cbd07af
Compare
cbd07af to
3121651
Compare
|
@SenseException Can I help you somehow with this PR? I rebased my branch onto 2.13.x and the test still fails if I undo my changes. |
This fixes a bug which arises in some cases when trying to delete two entities which have a circular reference through other entities. UnifOfWork computed the order which was not correct for deletions and it failed with ForeignKeyConstraintViolationException. To fix that, this adds a new particular function getCommitOrderForDeletions() to compute the right order. This function is similar to getCommitOrder(), but it operates only with entities set to be deleted. This also adds test for this issue.
3121651 to
992b8cc
Compare
|
Thank you for your offer and your patience. There's currently no help required that I know of. We'll come back in case this PR needs changes or maybe new test cases to prevent breaking changes. |
Co-authored-by: Josef Šupka <josef.supka@gmail.com>
Co-authored-by: Josef Šupka <josef.supka@gmail.com>
Co-authored-by: Josef Šupka <josef.supka@gmail.com>
Co-authored-by: Josef Šupka <josef.supka@gmail.com>
Co-authored-by: Josef Šupka <josef.supka@gmail.com>
This is part of the series of issues fixed by doctrine#10547. In particular, the changes from doctrine#10566 were relevant. See doctrine#9376 for the bug description; doctrine#9377 is obsoleted by doctrine#10547.
This is part of the series of issues fixed by doctrine#10547. In particular, the changes from doctrine#10566 were relevant. See doctrine#9376 for the bug description; doctrine#9377 is obsoleted by doctrine#10547.
This is part of the series of issues fixed by doctrine#10547. In particular, the changes from doctrine#10566 were relevant. See doctrine#9376 for the bug description; doctrine#9377 is obsoleted by doctrine#10547.
|
Also for me, the test case passes on the current 2.16.x branch without further modifications to the UoW. Yes, I am using a DBMS with foreign keys. Could you please re-check that and/or provide only the failing test in a dedicated PR against 2.16 (and notify me)? Thanks! /cc @greg0ire |
Fixes: #9376