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 doctrine#7877

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

@@ -643,17 +643,12 @@ protected function prepareUpdateData($entity)
                 continue;
             }
 
-            if ($newVal !== null) {
-                $oid = spl_object_hash($newVal);
-
-                if (isset($this->queuedInserts[$oid]) || $uow->isScheduledForInsert($newVal)) {
-                    // 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;
-                }
+            if ($this->isExtraUpdateRequired($entity, $newVal)) {
+                // 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;
             }
 
             $newValId = null;
@@ -681,6 +676,42 @@ protected function prepareUpdateData($entity)
         return $result;
     }
 
+    /**
+     * Decides if an extra update is required for the entity being persisted
+     * 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
+     *
+     * @param $entity
+     * @param $newVal
+     *
+     * @return bool
+     */
+    private function isExtraUpdateRequired($entity, $newVal): bool
+    {
+        if ($newVal === null) {
+           return false;
+        }
+        $oid = spl_object_hash($newVal);
+        $uow = $this->em->getUnitOfWork();
+        if (isset($this->queuedInserts[$oid]) || $uow->isScheduledForInsert($newVal)) {
+            if ($newVal !== $entity) {
+                return true;
+            } else {
+                $identifiers = $this->class->getIdentifier();
+                // Only single-column identifiers are supported
+                $entityChangeSet = $uow->getEntityChangeSet($entity);
+                if (\count($identifiers) === 1 && isset($entityChangeSet[$identifiers[0]])) {
+                    // Extra update is required if the current entity does not have yet a value for its identifier
+                    return ($entityChangeSet[$identifiers[0]][1] === null);
+                } else {
+                    return true;
+                }
+            }
+        } else {
+            return false;
+        }
+    }
+
     /**
      * Prepares the data changeset of a managed entity for database insertion (initial INSERT).
      * The changeset of the entity is obtained from the currently running UnitOfWork.

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

@@ -0,0 +1,127 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+class SelfReferencingTest extends OrmFunctionalTestCase
+{
+    protected function setUp() : void
+    {
+        parent::setUp();
+        $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);
+    }
+
+    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->parent->id);
+    }
+}
+
+/**
+ * @Entity
+ */
+class Issue7877ApplicationGenerated
+{
+    public function __construct(int $id)
+    {
+        $this->id = $id;
+    }
+
+    /**
+     * @Id
+     * @Column(type="integer")
+     * @GeneratedValue(strategy="NONE")
+     */
+    public $id;
+
+    /**
+     * @ManyToOne(targetEntity="Doctrine\Tests\ORM\Functional\Issue7877ApplicationGenerated")
+     */
+    public $parent;
+}
+
+/**
+ * @Entity
+ */
+class Issue7877DatabaseGenerated
+{
+    /**
+     * @Id
+     * @Column(type="integer")
+     * @GeneratedValue(strategy="AUTO")
+     */
+    public $id;
+
+    /**
+     * @ManyToOne(targetEntity="Doctrine\Tests\ORM\Functional\Issue7877DatabaseGenerated")
+     */
+    public $parent;
+}