From 8d4718f875b3ea142203ae4258ad4e91f1bacecb Mon Sep 17 00:00:00 2001 From: Mark Schmale Date: Thu, 22 Feb 2024 10:58:50 +0100 Subject: [PATCH 1/6] provides a test case for github issue 11154 After 2.17 (some?) EAGER fetched OneToMany associations stopped working, if they have multiple join columns. Loads for these associations will trigger a `MessingPositionalParameter` exception "Positional parameter at index 1 does not have a bound value". This test case should reproduce this issue, so it can be fixed. --- .../RootEntity.php | 56 +++++++++++++++++ .../SecondLevel.php | 62 +++++++++++++++++++ ...agerFetchOneToManyWithCompositeKeyTest.php | 31 ++++++++++ tests/Tests/OrmFunctionalTestCase.php | 9 +++ 4 files changed, 158 insertions(+) create mode 100644 tests/Tests/Models/EagerFetchedCompositeOneToMany/RootEntity.php create mode 100644 tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php create mode 100644 tests/Tests/ORM/Functional/EagerFetchOneToManyWithCompositeKeyTest.php diff --git a/tests/Tests/Models/EagerFetchedCompositeOneToMany/RootEntity.php b/tests/Tests/Models/EagerFetchedCompositeOneToMany/RootEntity.php new file mode 100644 index 00000000000..af16c686970 --- /dev/null +++ b/tests/Tests/Models/EagerFetchedCompositeOneToMany/RootEntity.php @@ -0,0 +1,56 @@ + + */ + private $secondLevel; + + public function __construct(int $id, string $other) + { + $this->otherKey = $other; + $this->secondLevel = new ArrayCollection(); + $this->id = $id; + } + + public function getId(): ?int + { + return $this->id; + } + + public function getOtherKey(): string + { + return $this->otherKey; + } +} diff --git a/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php b/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php new file mode 100644 index 00000000000..e67f3abc4fb --- /dev/null +++ b/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php @@ -0,0 +1,62 @@ +id = $id; + $this->upperId = $upper->getId(); + $this->otherKey = $upper->getOtherKey(); + $this->root = $upper; + } + + public function getId(): ?int + { + return $this->id; + } +} diff --git a/tests/Tests/ORM/Functional/EagerFetchOneToManyWithCompositeKeyTest.php b/tests/Tests/ORM/Functional/EagerFetchOneToManyWithCompositeKeyTest.php new file mode 100644 index 00000000000..8ad78cb253d --- /dev/null +++ b/tests/Tests/ORM/Functional/EagerFetchOneToManyWithCompositeKeyTest.php @@ -0,0 +1,31 @@ +useModelSet('eager_fetched_composite_one_to_many'); + + parent::setUp(); + } + + /** @ticket 11154 */ + public function testItDoesNotThrowAnExceptionWhenTriggeringALoad(): void + { + $a1 = new RootEntity(1, 'A'); + + $this->_em->persist($a1); + $this->_em->flush(); + + $this->_em->clear(); + + self::assertCount(1, $this->_em->getRepository(RootEntity::class)->findAll()); + } +} diff --git a/tests/Tests/OrmFunctionalTestCase.php b/tests/Tests/OrmFunctionalTestCase.php index 4a64136f184..ea2a0110405 100644 --- a/tests/Tests/OrmFunctionalTestCase.php +++ b/tests/Tests/OrmFunctionalTestCase.php @@ -342,6 +342,10 @@ abstract class OrmFunctionalTestCase extends OrmTestCase Models\Issue9300\Issue9300Child::class, Models\Issue9300\Issue9300Parent::class, ], + 'eager_fetched_composite_one_to_many' => [ + Models\EagerFetchedCompositeOneToMany\RootEntity::class, + Models\EagerFetchedCompositeOneToMany\SecondLevel::class, + ], ]; /** @param class-string ...$models */ @@ -671,6 +675,11 @@ protected function tearDown(): void $conn->executeStatement('DELETE FROM issue5989_managers'); } + if (isset($this->_usedModelSets['eager_fetched_composite_one_to_many'])) { + $conn->executeStatement('DELETE FROM eager_composite_join_second_level'); + $conn->executeStatement('DELETE FROM eager_composite_join_root'); + } + $this->_em->clear(); } From fcd02b1ee276635d4b39d55f492f7c8b0c1ebed4 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 16 Mar 2024 23:04:57 +0100 Subject: [PATCH 2/6] Cleanup tests not to use model sets. --- .../EagerFetchOneToManyWithCompositeKeyTest.php | 10 +++------- tests/Tests/OrmFunctionalTestCase.php | 9 --------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/tests/Tests/ORM/Functional/EagerFetchOneToManyWithCompositeKeyTest.php b/tests/Tests/ORM/Functional/EagerFetchOneToManyWithCompositeKeyTest.php index 8ad78cb253d..82b9d0b8acb 100644 --- a/tests/Tests/ORM/Functional/EagerFetchOneToManyWithCompositeKeyTest.php +++ b/tests/Tests/ORM/Functional/EagerFetchOneToManyWithCompositeKeyTest.php @@ -5,20 +5,16 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\Tests\Models\EagerFetchedCompositeOneToMany\RootEntity; +use Doctrine\Tests\Models\EagerFetchedCompositeOneToMany\SecondLevel; use Doctrine\Tests\OrmFunctionalTestCase; final class EagerFetchOneToManyWithCompositeKeyTest extends OrmFunctionalTestCase { - protected function setUp(): void - { - $this->useModelSet('eager_fetched_composite_one_to_many'); - - parent::setUp(); - } - /** @ticket 11154 */ public function testItDoesNotThrowAnExceptionWhenTriggeringALoad(): void { + $this->setUpEntitySchema([RootEntity::class, SecondLevel::class]); + $a1 = new RootEntity(1, 'A'); $this->_em->persist($a1); diff --git a/tests/Tests/OrmFunctionalTestCase.php b/tests/Tests/OrmFunctionalTestCase.php index ea2a0110405..4a64136f184 100644 --- a/tests/Tests/OrmFunctionalTestCase.php +++ b/tests/Tests/OrmFunctionalTestCase.php @@ -342,10 +342,6 @@ abstract class OrmFunctionalTestCase extends OrmTestCase Models\Issue9300\Issue9300Child::class, Models\Issue9300\Issue9300Parent::class, ], - 'eager_fetched_composite_one_to_many' => [ - Models\EagerFetchedCompositeOneToMany\RootEntity::class, - Models\EagerFetchedCompositeOneToMany\SecondLevel::class, - ], ]; /** @param class-string ...$models */ @@ -675,11 +671,6 @@ protected function tearDown(): void $conn->executeStatement('DELETE FROM issue5989_managers'); } - if (isset($this->_usedModelSets['eager_fetched_composite_one_to_many'])) { - $conn->executeStatement('DELETE FROM eager_composite_join_second_level'); - $conn->executeStatement('DELETE FROM eager_composite_join_root'); - } - $this->_em->clear(); } From 820a0da4c188fb96de6782fef53ed5639ff690b9 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 16 Mar 2024 23:05:28 +0100 Subject: [PATCH 3/6] Do not schedule batch loading for target classes with composite identifier. --- src/UnitOfWork.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index 00698e56c60..c5996613b48 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -3169,9 +3169,9 @@ public function createEntity($className, array $data, &$hints = []) if ($hints['fetchMode'][$class->name][$field] === ClassMetadata::FETCH_EAGER) { $isIteration = isset($hints[Query::HINT_INTERNAL_ITERATION]) && $hints[Query::HINT_INTERNAL_ITERATION]; - if (! $isIteration && $assoc['type'] === ClassMetadata::ONE_TO_MANY) { + if ($assoc['type'] === ClassMetadata::ONE_TO_MANY && ! $isIteration && ! $targetClass->isIdentifierComposite) { $this->scheduleCollectionForBatchLoading($pColl, $class); - } elseif (($isIteration && $assoc['type'] === ClassMetadata::ONE_TO_MANY) || $assoc['type'] === ClassMetadata::MANY_TO_MANY) { + } else { $this->loadCollection($pColl); $pColl->takeSnapshot(); } From 1622b7877dcd8db015ff3d99fd662e0bc506ff37 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 17 Mar 2024 18:02:11 +0100 Subject: [PATCH 4/6] Fix entities and mapping. --- .../SecondLevel.php | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php b/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php index e67f3abc4fb..121a57ecc4c 100644 --- a/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php +++ b/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php @@ -12,14 +12,6 @@ */ class SecondLevel { - /** - * @ORM\Id - * @ORM\Column(type="integer", nullable=false) - * - * @var int|null - */ - private $id = null; - /** * @ORM\Id * @ORM\Column(type="integer", nullable=false) @@ -39,17 +31,16 @@ class SecondLevel /** * @ORM\ManyToOne(targetEntity=RootEntity::class, inversedBy="secondLevel") * @ORM\JoinColumns({ - * @ORM\JoinColumn(name="other_key", referencedColumnName="other_key"), - * @ORM\JoinColumn(name="upper_id", referencedColumnName="id") + * @ORM\JoinColumn(name="root_other_key", referencedColumnName="other_key"), + * @ORM\JoinColumn(name="root_id", referencedColumnName="id") * }) * * @var RootEntity */ private $root; - public function __construct(int $id, RootEntity $upper) + public function __construct(RootEntity $upper) { - $this->id = $id; $this->upperId = $upper->getId(); $this->otherKey = $upper->getOtherKey(); $this->root = $upper; From 5e6d5c06a987c073f01128577c78102dd468a608 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 17 Mar 2024 19:43:26 +0100 Subject: [PATCH 5/6] Key on fk --- .../Models/EagerFetchedCompositeOneToMany/SecondLevel.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php b/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php index 121a57ecc4c..99fed077723 100644 --- a/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php +++ b/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php @@ -8,7 +8,9 @@ /** * @ORM\Entity - * @ORM\Table(name="eager_composite_join_second_level") + * @ORM\Table(name="eager_composite_join_second_level", indexes={ + * @ORM\Index(name="root_other_key_idx", columns={"root_other_key", "root_id"}) + * }) */ class SecondLevel { From 3e3c023c95d6c7c848a1bcd71180baf7e033743a Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 17 Mar 2024 19:50:56 +0100 Subject: [PATCH 6/6] Switch join columns around, otherwise index doesnt match --- .../Models/EagerFetchedCompositeOneToMany/SecondLevel.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php b/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php index 99fed077723..33f878c8f41 100644 --- a/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php +++ b/tests/Tests/Models/EagerFetchedCompositeOneToMany/SecondLevel.php @@ -33,8 +33,8 @@ class SecondLevel /** * @ORM\ManyToOne(targetEntity=RootEntity::class, inversedBy="secondLevel") * @ORM\JoinColumns({ - * @ORM\JoinColumn(name="root_other_key", referencedColumnName="other_key"), - * @ORM\JoinColumn(name="root_id", referencedColumnName="id") + * @ORM\JoinColumn(name="root_id", referencedColumnName="id"), + * @ORM\JoinColumn(name="root_other_key", referencedColumnName="other_key") * }) * * @var RootEntity