Skip to content

Fix JoinedSubclassPersister when multiple entities are inserted#11934

Merged
greg0ire merged 1 commit intodoctrine:2.20.xfrom
mvorisek:fix_joined_subclass_persister_insert_of_multiple_entities
Aug 2, 2025
Merged

Fix JoinedSubclassPersister when multiple entities are inserted#11934
greg0ire merged 1 commit intodoctrine:2.20.xfrom
mvorisek:fix_joined_subclass_persister_insert_of_multiple_entities

Conversation

@mvorisek
Copy link
Copy Markdown
Contributor

@mvorisek mvorisek commented May 8, 2025

Same fix as in base/BasicEntityPersister class - https://github.com/doctrine/orm/blob/2.20.3/src/Persisters/Entity/BasicEntityPersister.php#L310. Missed in #10735.

I have extracted this from reworked #8260 which will target v3.x.

The fix can be verified by modifying UnitOfWork to execute BasicEntityPersister::executeInserts() for multiple entities at once for the same entity class/persister instance - https://github.com/doctrine/orm/blob/2.20.3/src/UnitOfWork.php#L1186 - then reproducible on Doctrine\Tests\ORM\Functional\Ticket\GH10531Test::testInserts test.

As extending/modifying UnitOfWork in tests in not easily possible, I submit this fix for v2.x without a test.

@mvorisek
Copy link
Copy Markdown
Contributor Author

@greg0ire can this PR be merged please?

Test for this cannot be easily added (it will be very long), I hope the description is enough. If you require better commit description please let me know and what exactly should I mention there. Thank you.

@greg0ire
Copy link
Copy Markdown
Member

Please include a description of the problem and a description of the fix in the commit message

@mvorisek mvorisek force-pushed the fix_joined_subclass_persister_insert_of_multiple_entities branch from 06e6d19 to c32852b Compare July 20, 2025 22:48
@mvorisek
Copy link
Copy Markdown
Contributor Author

I have adjusted the commit message.

@greg0ire
Copy link
Copy Markdown
Member

You've copied the PR description to the commit message.

Fix JoinedSubclassPersister as BasicEntityPersister was already fixed in #10735.

does not describe the problem, and delegates the description of the fix to another page

I have extracted this from reworked #8260 which will target v3.x.

A reference, again, but no description

The fix can be verified by modifying UnitOfWork to execute BasicEntityPersister::executeInserts() for multiple entities at once for the same entity class/persister instance - https://github.com/doctrine/orm/blob/2.20.3/src/UnitOfWork.php#L1186 - then reproducible on Doctrine\Tests\ORM\Functional\Ticket\GH10531Test::testInserts test.

Describes neither the problem nor the fix

As extending/modifying UnitOfWork in tests in not easily possible, I submit this fix for v2.x without a test.

Same

@mvorisek
Copy link
Copy Markdown
Contributor Author

In my fork I work on #8260. I have discovered this issue when working with the ˙JoinedSubclassPersister˙ with multiple entities at once. In #10735 the exact change was done for the basic BasicEntityPersister persister and there is an explanatory comment.

So the exact same reasoning goes for this PR. Should I copy the same comment above the unset here too?

I hope this reasons the change enough. Let me know if I can/should do something more.

@mvorisek
Copy link
Copy Markdown
Contributor Author

mvorisek commented Aug 1, 2025

@greg0ire the issue is described in https://github.com/doctrine/orm/blob/2.20.3/src/Persisters/Entity/BasicEntityPersister.php#L303-L309 . Should I copy the comment here too? Or add above the unset like to this comment? I would like to finish this PR, please let me know what to do.

@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Aug 1, 2025

Yes, you should copy the comment above the unset, if it's needed in BasicEntityPersister, then it must be needed where you act as well.

Fix JoinedSubclassPersister as BasicEntityPersister was already fixed in doctrineGH-10735.

The fix can be verified by modifying UnitOfWork to execute `BasicEntityPersister::executeInserts()` for multiple entities at once for the same entity class/persister instance - https://github.com/doctrine/orm/blob/2.20.3/src/UnitOfWork.php#L1186 - then reproducible on `Doctrine\Tests\ORM\Functional\Ticket\GH10531Test::testInserts` test.

As extending/modifying UnitOfWork in tests in not easily possible, I submit this fix for v2.x without a test.
@mvorisek mvorisek force-pushed the fix_joined_subclass_persister_insert_of_multiple_entities branch from c32852b to 97a7cb8 Compare August 1, 2025 13:31
@mvorisek
Copy link
Copy Markdown
Contributor Author

mvorisek commented Aug 1, 2025

done and improved the base comment slightly

@greg0ire greg0ire added this to the 2.20.6 milestone Aug 2, 2025
@greg0ire greg0ire merged commit 7449571 into doctrine:2.20.x Aug 2, 2025
74 checks passed
@mvorisek mvorisek deleted the fix_joined_subclass_persister_insert_of_multiple_entities branch August 2, 2025 07:09
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