Skip to content

Commit 469d49b

Browse files
sylfabresylfabre
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 e9e012a commit 469d49b

File tree

2 files changed

+165
-11
lines changed

2 files changed

+165
-11
lines changed

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

+44-11
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@
4040
use function array_map;
4141
use function array_merge;
4242
use function assert;
43+
use function count;
4344
use function reset;
45+
use function spl_object_hash;
4446

4547
/**
4648
* A BasicEntityPersister maps an entity to a single table in a relational database.
@@ -643,17 +645,12 @@ protected function prepareUpdateData($entity)
643645
continue;
644646
}
645647

646-
if ($newVal !== null) {
647-
$oid = spl_object_hash($newVal);
648-
649-
if (isset($this->queuedInserts[$oid]) || $uow->isScheduledForInsert($newVal)) {
650-
// The associated entity $newVal is not yet persisted, so we must
651-
// set $newVal = null, in order to insert a null value and schedule an
652-
// extra update on the UnitOfWork.
653-
$uow->scheduleExtraUpdate($entity, [$field => [null, $newVal]]);
654-
655-
$newVal = null;
656-
}
648+
if ($this->isExtraUpdateRequired($entity, $newVal)) {
649+
// The associated entity $newVal is not yet persisted, so we must
650+
// set $newVal = null, in order to insert a null value and schedule an
651+
// extra update on the UnitOfWork.
652+
$uow->scheduleExtraUpdate($entity, [$field => [null, $newVal]]);
653+
$newVal = null;
657654
}
658655

659656
$newValId = null;
@@ -681,6 +678,42 @@ protected function prepareUpdateData($entity)
681678
return $result;
682679
}
683680

681+
/**
682+
* Decides if an extra update is required for the entity being persisted
683+
* This is only required if the associated entity is different from the current one,
684+
* and if the current entity relies on the database to generate its id
685+
*
686+
* @param mixed $entity
687+
* @param mixed $newVal
688+
*
689+
* @return bool Returns true is an extra update is required
690+
*/
691+
private function isExtraUpdateRequired($entity, $newVal) : bool
692+
{
693+
if ($newVal === null) {
694+
return false;
695+
}
696+
$oid = spl_object_hash($newVal);
697+
$uow = $this->em->getUnitOfWork();
698+
if (isset($this->queuedInserts[$oid]) || $uow->isScheduledForInsert($newVal)) {
699+
if ($newVal !== $entity) {
700+
return true;
701+
} else {
702+
$identifiers = $this->class->getIdentifier();
703+
// Only single-column identifiers are supported
704+
$entityChangeSet = $uow->getEntityChangeSet($entity);
705+
if (count($identifiers) === 1 && isset($entityChangeSet[$identifiers[0]])) {
706+
// Extra update is required if the current entity does not have yet a value for its identifier
707+
return $entityChangeSet[$identifiers[0]][1] === null;
708+
} else {
709+
return true;
710+
}
711+
}
712+
} else {
713+
return false;
714+
}
715+
}
716+
684717
/**
685718
* Prepares the data changeset of a managed entity for database insertion (initial INSERT).
686719
* The changeset of the entity is obtained from the currently running UnitOfWork.

‎tests/Doctrine/Tests/ORM/Functional/SelfReferencingTest.php

+121
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional;
6+
7+
use Doctrine\Tests\OrmFunctionalTestCase;
8+
use function count;
9+
10+
class SelfReferencingTest extends OrmFunctionalTestCase
11+
{
12+
protected function setUp() : void
13+
{
14+
parent::setUp();
15+
$classMetadatas = [
16+
$this->_em->getClassMetadata(Issue7877ApplicationGenerated::class),
17+
$this->_em->getClassMetadata(Issue7877DatabaseGenerated::class),
18+
];
19+
// We first drop the schema to avoid collision between tests
20+
$this->_schemaTool->dropSchema($classMetadatas);
21+
$this->_schemaTool->createSchema($classMetadatas);
22+
}
23+
24+
public function providerDifferentEntity()
25+
{
26+
yield [Issue7877ApplicationGenerated::class];
27+
yield [Issue7877DatabaseGenerated::class];
28+
}
29+
30+
/**
31+
* @dataProvider providerDifferentEntity
32+
*/
33+
public function testExtraUpdateWithDifferentEntities(string $class)
34+
{
35+
$parent = new $class($parentId = 1);
36+
$this->_em->persist($parent);
37+
38+
$child = new $class($childId = 2);
39+
$child->parent = $parent;
40+
$this->_em->persist($child);
41+
42+
$count = count($this->_sqlLoggerStack->queries);
43+
$this->_em->flush();
44+
$this->assertCount($count + 5, $this->_sqlLoggerStack->queries);
45+
46+
$this->_em->clear();
47+
48+
$child = $this->_em->find($class, $childId);
49+
$this->assertSame($parentId, $child->parent->id);
50+
}
51+
52+
public function testNoExtraUpdateWithApplicationGeneratedId()
53+
{
54+
$entity = new Issue7877ApplicationGenerated($entityId = 1);
55+
$entity->parent = $entity;
56+
$this->_em->persist($entity);
57+
58+
$count = count($this->_sqlLoggerStack->queries);
59+
$this->_em->flush();
60+
$this->assertCount($count + 3, $this->_sqlLoggerStack->queries);
61+
62+
$this->_em->clear();
63+
64+
$child = $this->_em->find(Issue7877ApplicationGenerated::class, $entityId);
65+
$this->assertSame($entityId, $child->parent->id);
66+
}
67+
68+
public function textExtraUpdateWithDatabaseGeneratedId()
69+
{
70+
$entity = new Issue7877DatabaseGenerated();
71+
$entity->parent = $entity;
72+
$this->_em->persist($entity);
73+
74+
$count = count($this->_sqlLoggerStack->queries);
75+
$this->_em->flush();
76+
$this->assertCount($count + 4, $this->_sqlLoggerStack->queries);
77+
$entityId = $entity->id;
78+
79+
$this->_em->clear();
80+
81+
$child = $this->_em->find(Issue7877DatabaseGenerated::class, $entityId);
82+
$this->assertSame($entityId, $child->parent->id);
83+
}
84+
}
85+
86+
/**
87+
* @Entity
88+
*/
89+
class Issue7877ApplicationGenerated
90+
{
91+
public function __construct(int $id)
92+
{
93+
$this->id = $id;
94+
}
95+
96+
/**
97+
* @Id
98+
* @Column(type="integer")
99+
* @GeneratedValue(strategy="NONE")
100+
*/
101+
public $id;
102+
103+
/** @ManyToOne(targetEntity="Doctrine\Tests\ORM\Functional\Issue7877ApplicationGenerated") */
104+
public $parent;
105+
}
106+
107+
/**
108+
* @Entity
109+
*/
110+
class Issue7877DatabaseGenerated
111+
{
112+
/**
113+
* @Id
114+
* @Column(type="integer")
115+
* @GeneratedValue(strategy="AUTO")
116+
*/
117+
public $id;
118+
119+
/** @ManyToOne(targetEntity="Doctrine\Tests\ORM\Functional\Issue7877DatabaseGenerated") */
120+
public $parent;
121+
}

0 commit comments

Comments
 (0)