Skip to content

Commit c2b3c7e

Browse files
committed
Optimize INSERT query to avoid an extra UPDATE
BaseEntityPersister can save an extra UPDATE query when it persists a self-referencing entity using an application generated identifier Fixes #7877
1 parent d528f70 commit c2b3c7e

File tree

2 files changed

+194
-3
lines changed

2 files changed

+194
-3
lines changed

lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php

+21-3
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,8 @@ protected function prepareUpdateData($entity)
639639
: null;
640640

641641
// @todo guilhermeblanco This should check column insertability/updateability instead of field changeset
642-
foreach ($uow->getEntityChangeSet($entity) as $propertyName => $propertyChangeSet) {
642+
$entityChangeSet = $uow->getEntityChangeSet($entity);
643+
foreach ($entityChangeSet as $propertyName => $propertyChangeSet) {
643644
if ($versionPropertyName === $propertyName) {
644645
continue;
645646
}
@@ -665,9 +666,26 @@ protected function prepareUpdateData($entity)
665666
// set $newVal = null, in order to insert a null value and schedule an
666667
// extra update on the UnitOfWork.
667668
if ($newValue !== null && $uow->isScheduledForInsert($newValue)) {
668-
$uow->scheduleExtraUpdate($entity, [$propertyName => [null, $newValue]]);
669+
// This is only required if the associated entity is different from the current one,
670+
// and if the current entity relies on the database to generate its id
671+
if ($newValue !== $entity) {
672+
$scheduleExtraUpdate = true;
673+
} else {
674+
$identifiers = $this->class->getIdentifier();
675+
// Only single-column identifiers are supported
676+
if (1 === count($identifiers) && isset($entityChangeSet[$identifiers[0]])) {
677+
// Extra update is required if the current entity does not have yet a value for its identifier
678+
$scheduleExtraUpdate = ($entityChangeSet[$identifiers[0]][1] === null);
679+
} else {
680+
$scheduleExtraUpdate = true;
681+
}
682+
}
683+
684+
if ($scheduleExtraUpdate) {
685+
$uow->scheduleExtraUpdate($entity, [$propertyName => [null, $newValue]]);
686+
$newValue = null;
687+
}
669688

670-
$newValue = null;
671689
}
672690

673691
$targetClass = $this->em->getClassMetadata($property->getTargetEntity());
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional;
6+
7+
use Doctrine\ORM\Annotation as ORM;
8+
use Doctrine\Tests\OrmFunctionalTestCase;
9+
10+
class SelfReferencingTest extends OrmFunctionalTestCase
11+
{
12+
protected function setUp() : void
13+
{
14+
parent::setUp();
15+
try {
16+
$classMetadatas = [
17+
$this->em->getClassMetadata(Issue7877ApplicationGenerated::class),
18+
$this->em->getClassMetadata(Issue7877DatabaseGenerated::class),
19+
];
20+
// We first drop the schema to avoid collision between tests
21+
$this->schemaTool->dropSchema($classMetadatas);
22+
$this->schemaTool->createSchema($classMetadatas);
23+
} catch (Exception $e) {
24+
}
25+
}
26+
27+
public function providerDifferentEntity()
28+
{
29+
yield [Issue7877ApplicationGenerated::class];
30+
yield [Issue7877DatabaseGenerated::class];
31+
}
32+
33+
/**
34+
* @dataProvider providerDifferentEntity
35+
*/
36+
public function testDifferentEntity(string $class)
37+
{
38+
$count = count($this->sqlLoggerStack->queries);
39+
40+
$parent = new $class($parentId = 1);
41+
$this->em->persist($parent);
42+
43+
$child = new $class($childId = 2);
44+
$child->setParent($parent);
45+
$this->em->persist($child);
46+
47+
$this->em->flush();
48+
$this->assertCount($count + 4, $this->sqlLoggerStack->queries);
49+
50+
$this->em->clear();
51+
52+
$child = $this->em->find($class, $childId);
53+
$this->assertSame($parentId, $child->getParent()->getId());
54+
}
55+
56+
public function testSameEntityApplicationGenerated()
57+
{
58+
$count = count($this->sqlLoggerStack->queries);
59+
60+
$entity = new Issue7877ApplicationGenerated($entityId = 1);
61+
$entity->setParent($entity);
62+
$this->em->persist($entity);
63+
64+
$this->em->flush();
65+
$this->assertCount($count + 3, $this->sqlLoggerStack->queries);
66+
67+
$this->em->clear();
68+
69+
$child = $this->em->find(Issue7877ApplicationGenerated::class, $entityId);
70+
$this->assertSame($entityId, $child->getParent()->getId());
71+
}
72+
73+
public function testSameEntityDatabaseGenerated()
74+
{
75+
$count = count($this->sqlLoggerStack->queries);
76+
77+
$entity = new Issue7877DatabaseGenerated();
78+
$entity->setParent($entity);
79+
$this->em->persist($entity);
80+
81+
$this->em->flush();
82+
$this->assertCount($count + 4, $this->sqlLoggerStack->queries);
83+
$entityId = $entity->getId();
84+
85+
$this->em->clear();
86+
87+
$child = $this->em->find(Issue7877DatabaseGenerated::class, $entityId);
88+
$this->assertSame($entityId, $child->getParent()->getId());
89+
}
90+
}
91+
92+
/**
93+
* @ORM\Entity
94+
*/
95+
class Issue7877ApplicationGenerated
96+
{
97+
public function __construct(int $id)
98+
{
99+
$this->id = $id;
100+
}
101+
102+
/**
103+
* @ORM\Id
104+
* @ORM\Column(type="integer")
105+
* @ORM\GeneratedValue(strategy="NONE")
106+
*/
107+
private $id;
108+
109+
public function getId(): ?int
110+
{
111+
return $this->id;
112+
}
113+
114+
public function setId(int $id)
115+
{
116+
$this->id = $id;
117+
}
118+
119+
/**
120+
* @var self
121+
* @ORM\ManyToOne(targetEntity="Doctrine\Tests\ORM\Functional\Issue7877ApplicationGenerated")
122+
*/
123+
private $parent;
124+
125+
public function getParent(): ?self
126+
{
127+
return $this->parent;
128+
}
129+
130+
public function setParent(self $parent)
131+
{
132+
$this->parent = $parent;
133+
}
134+
}
135+
136+
/**
137+
* @ORM\Entity
138+
*/
139+
class Issue7877DatabaseGenerated
140+
{
141+
/**
142+
* @ORM\Id
143+
* @ORM\Column(type="integer")
144+
* @ORM\GeneratedValue(strategy="AUTO")
145+
*/
146+
private $id;
147+
148+
public function getId(): ?int
149+
{
150+
return $this->id;
151+
}
152+
153+
public function setId(int $id)
154+
{
155+
$this->id = $id;
156+
}
157+
158+
/**
159+
* @var self
160+
* @ORM\ManyToOne(targetEntity="Doctrine\Tests\ORM\Functional\Issue7877DatabaseGenerated")
161+
*/
162+
private $parent;
163+
164+
public function getParent(): ?self
165+
{
166+
return $this->parent;
167+
}
168+
169+
public function setParent(self $parent)
170+
{
171+
$this->parent = $parent;
172+
}
173+
}

0 commit comments

Comments
 (0)