Compute particular commit order for deletions #9377
Conversation
jusephe
force-pushed
the
commit-order-test
branch
from
c47ddcc
to
dbd630e
Compare
|
I just pushed new commit to fix "Static Analysis with Psalm". |
jusephe
force-pushed
the
commit-order-test
branch
from
dbd630e
to
c58dea4
Compare
jusephe
force-pushed
the
commit-order-test
branch
from
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... |
jusephe
force-pushed
the
commit-order-test
branch
2 times, most recently
from
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. |
jusephe
force-pushed
the
commit-order-test
branch
from
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). |
jusephe
force-pushed
the
commit-order-test
branch
from
43f1e3e
to
cbd07af
Compare
jusephe
force-pushed
the
commit-order-test
branch
from
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.
jusephe
force-pushed
the
commit-order-test
branch
from
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