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

Author: sylfabre

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

@@ -614,7 +614,8 @@ protected function prepareUpdateData($entity)
             $versionField = $this->class->versionField;
         }
 
-        foreach ($uow->getEntityChangeSet($entity) as $field => $change) {
+        $entityChangeSet = $uow->getEntityChangeSet($entity);
+        foreach ($entityChangeSet as $field => $change) {
             if (isset($versionField) && $versionField == $field) {
                 continue;
             }
@@ -650,9 +651,25 @@ protected function prepareUpdateData($entity)
                     // The associated entity $newVal is not yet persisted, so we must
                     // set $newVal = null, in order to insert a null value and schedule an
                     // extra update on the UnitOfWork.
-                    $uow->scheduleExtraUpdate($entity, [$field => [null, $newVal]]);
-
-                    $newVal = null;
+                    // This is only required if the associated entity is different from the current one,
+	                // and if the current entity relies on the database to generate its id
+	                if ($newVal !== $entity) {
+	                    $scheduleExtraUpdate = true;
+	                } else {
+	                    $identifiers = $this->class->getIdentifier();
+	                    // Only single-column identifiers are supported
+	                    if (1 === count($identifiers) && isset($entityChangeSet[$identifiers[0]])) {
+	                        // Extra update is required if the current entity does not have yet a value for its identifier
+	                        $scheduleExtraUpdate = ($entityChangeSet[$identifiers[0]][1] === null);
+	                    } else {
+	                        $scheduleExtraUpdate = true;
+	                    }
+	                }
+
+	                if ($scheduleExtraUpdate) {
+	                    $uow->scheduleExtraUpdate($entity, [$field => [null, $newVal]]);
+	                    $newVal = null;
+	                }
                 }
             }
 

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

@@ -0,0 +1,133 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\ORM\Annotation as ORM;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+class SelfReferencingTest extends OrmFunctionalTestCase
+{
+    protected function setUp() : void
+    {
+        parent::setUp();
+        try {
+            $classMetadatas = [
+                $this->em->getClassMetadata(Issue7877ApplicationGenerated::class),
+                $this->em->getClassMetadata(Issue7877DatabaseGenerated::class),
+            ];
+            // We first drop the schema to avoid collision between tests
+            $this->schemaTool->dropSchema($classMetadatas);
+            $this->schemaTool->createSchema($classMetadatas);
+        } catch (Exception $e) {
+        }
+    }
+
+    public function providerDifferentEntity()
+    {
+        yield [Issue7877ApplicationGenerated::class];
+        yield [Issue7877DatabaseGenerated::class];
+    }
+
+    /**
+     * @dataProvider providerDifferentEntity
+     */
+    public function testExtraUpdateWithDifferentEntities(string $class)
+    {
+        $count = count($this->sqlLoggerStack->queries);
+
+        $parent = new $class($parentId = 1);
+        $this->em->persist($parent);
+
+        $child = new $class($childId = 2);
+        $child->parent = $parent;
+        $this->em->persist($child);
+
+        $this->em->flush();
+        $this->assertCount($count + 4, $this->sqlLoggerStack->queries);
+
+        $this->em->clear();
+
+        $child = $this->em->find($class, $childId);
+        $this->assertSame($parentId, $child->parent->id);
+    }
+
+    public function testNoExtraUpdateWithApplicationGeneratedId()
+    {
+        $count = count($this->sqlLoggerStack->queries);
+
+        $entity = new Issue7877ApplicationGenerated($entityId = 1);
+        $entity->parent = $entity;
+        $this->em->persist($entity);
+
+        $this->em->flush();
+        $this->assertCount($count + 3, $this->sqlLoggerStack->queries);
+
+        $this->em->clear();
+
+        $child = $this->em->find(Issue7877ApplicationGenerated::class, $entityId);
+        $this->assertSame($entityId, $child->parent->id);
+    }
+
+    public function textExtraUpdateWithDatabaseGeneratedId()
+    {
+        $count = count($this->sqlLoggerStack->queries);
+
+        $entity = new Issue7877DatabaseGenerated();
+        $entity->parent = $entity;
+        $this->em->persist($entity);
+
+        $this->em->flush();
+        $this->assertCount($count + 4, $this->sqlLoggerStack->queries);
+        $entityId = $entity->id;
+
+        $this->em->clear();
+
+        $child = $this->em->find(Issue7877DatabaseGenerated::class, $entityId);
+        $this->assertSame($entityId, $child->paren->id);
+    }
+}
+
+/**
+ * @ORM\Entity
+ */
+class Issue7877ApplicationGenerated
+{
+    public function __construct(int $id)
+    {
+        $this->id = $id;
+    }
+
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue(strategy="NONE")
+     */
+    public $id;
+
+    /**
+     * @var self
+     * @ORM\ManyToOne(targetEntity="Doctrine\Tests\ORM\Functional\Issue7877ApplicationGenerated")
+     */
+    public $parent;
+}
+
+/**
+ * @ORM\Entity
+ */
+class Issue7877DatabaseGenerated
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue(strategy="AUTO")
+     */
+    public $id;
+
+    /**
+     * @var self
+     * @ORM\ManyToOne(targetEntity="Doctrine\Tests\ORM\Functional\Issue7877DatabaseGenerated")
+     */
+    public $parent;
+}