Merge remote-tracking branch 'derrabus/3.0.x' into 3.0.x

* derrabus/3.0.x:
  Deprecate annotation classes for named queries
  Fix typos
  Housekeeping: Revert change to AbstractExporter, not needed without subselect fetch.
  Address review comments.
  Explain internals of eager loading in a bit more detail and how its configured.
  1:1 and M:1 associations also use fetch batch size configuration now.
  Add another testcase for DQL based fetch eager of collection.
  last violation hopefully
  Static analysis
  Housekeeping: phpcs
  Directly load many to many collections, batching not supported yet. fix tests.
  Avoid new fetch mode, use this strategy with fetch=EAGER for collections.
  Make sure to many assocatinos are also respecting AbstractQuery::setFetchMode
  Disallow use of fetch=SUBSELECT on to-one associations.
  Go through Persister API instead of indirectly through repository.
  Introduce configuration option for subselect batch size.
  Houskeeping: phpcs
  Disallow WITH keyword on fetch joined associatiosn via subselect.
  [GH-1569] Add new SUBSELECT fetch mode for OneToMany associations.

Author: derrabus

UPGRADE.md

@@ -666,6 +666,15 @@ Use `toIterable()` instead.
 
 # Upgrade to 2.17
 
+## Deprecate annotations classes for named queries
+
+The following classes have been deprecated:
+
+* `Doctrine\ORM\Mapping\NamedNativeQueries`
+* `Doctrine\ORM\Mapping\NamedNativeQuery`
+* `Doctrine\ORM\Mapping\NamedQueries`
+* `Doctrine\ORM\Mapping\NamedQuery`
+
 ## Deprecate `Doctrine\ORM\Query\Exec\AbstractSqlExecutor::_sqlStatements`
 
 Use `Doctrine\ORM\Query\Exec\AbstractSqlExecutor::sqlStatements` instead.

docs/en/reference/working-with-objects.rst

@@ -711,6 +711,23 @@ and these associations are mapped as EAGER, they will automatically
 be loaded together with the entity being queried and is thus
 immediately available to your application.
 
+Eager Loading can also be configured at runtime through
+``AbstractQuery::setFetchMode`` in DQL or Native Queries.
+
+Eager loading for many-to-one and one-to-one associations is using either a
+LEFT JOIN or a second query for fetching the related entity eagerly.
+
+Eager loading for many-to-one associations uses a second query to load
+the collections for several entities at the same time.
+
+When many-to-many, one-to-one or one-to-many associations are eagerly loaded,
+then the global batch size configuration is used to avoid IN(?) queries with
+too many arguments. The default batch size is 100 and can be changed with
+``Configuration::setEagerFetchBatchSize()``.
+
+For eagerly loaded Many-To-Many associations one query has to be made for each
+collection.
+
 By Lazy Loading
 ~~~~~~~~~~~~~~~
 

lib/Doctrine/ORM/Configuration.php

@@ -636,4 +636,14 @@ public function isRejectIdCollisionInIdentityMapEnabled(): bool
     {
         return true;
     }
+
+    public function setEagerFetchBatchSize(int $batchSize = 100): void
+    {
+        $this->attributes['fetchModeSubselectBatchSize'] = $batchSize;
+    }
+
+    public function getEagerFetchBatchSize(): int
+    {
+        return $this->attributes['fetchModeSubselectBatchSize'] ?? 100;
+    }
 }

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

@@ -1220,7 +1220,7 @@ protected function getSelectColumnsSQL(): string
             }
 
             $isAssocToOneInverseSide = $assoc->isToOne() && ! $assoc->isOwningSide();
-            $isAssocFromOneEager     = ! $assoc->isManyToMany() && $assoc->fetch === ClassMetadata::FETCH_EAGER;
+            $isAssocFromOneEager     = $assoc->isToOne() && $assoc->fetch === ClassMetadata::FETCH_EAGER;
 
             if (! ($isAssocFromOneEager || $isAssocToOneInverseSide)) {
                 continue;

lib/Doctrine/ORM/Query/QueryException.php

@@ -117,6 +117,14 @@ public static function iterateWithFetchJoinNotAllowed(AssociationMapping $assoc)
         );
     }
 
+    public static function eagerFetchJoinWithNotAllowed(string $sourceEntity, string $fieldName): self
+    {
+        return new self(
+            'Associations with fetch-mode=EAGER may not be using WITH conditions in
+             "' . $sourceEntity . '#' . $fieldName . '".',
+        );
+    }
+
     public static function iterateWithMixedResultNotAllowed(): self
     {
         return new self('Iterating a query with mixed results (using scalars) is not supported.');

lib/Doctrine/ORM/Query/SqlWalker.php

@@ -894,7 +894,9 @@ public function walkJoinAssociationDeclaration(
             }
         }
 
-        $targetTableJoin = null;
+        if ($relation->fetch === ClassMetadata::FETCH_EAGER && $condExpr !== null) {
+            throw QueryException::eagerFetchJoinWithNotAllowed($assoc->sourceEntity, $assoc->fieldName);
+        }
 
         // This condition is not checking ClassMetadata::MANY_TO_ONE, because by definition it cannot
         // be the owning side and previously we ensured that $assoc is always the owning side of the associations.

lib/Doctrine/ORM/UnitOfWork.php

@@ -33,6 +33,7 @@
 use Doctrine\ORM\Mapping\AssociationMapping;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\MappingException;
+use Doctrine\ORM\Mapping\ToManyInverseSideMapping;
 use Doctrine\ORM\Persisters\Collection\CollectionPersister;
 use Doctrine\ORM\Persisters\Collection\ManyToManyPersister;
 use Doctrine\ORM\Persisters\Collection\OneToManyPersister;
@@ -50,6 +51,7 @@
 use Throwable;
 use UnexpectedValueException;
 
+use function array_chunk;
 use function array_combine;
 use function array_diff_key;
 use function array_filter;
@@ -292,6 +294,9 @@ class UnitOfWork implements PropertyChangedListener
      */
     private array $eagerLoadingEntities = [];
 
+    /** @var array<string, array<string, mixed>> */
+    private array $eagerLoadingCollections = [];
+
     protected bool $hasCache = false;
 
     /**
@@ -2218,6 +2223,7 @@ public function clear(): void
         $this->pendingCollectionElementRemovals =
         $this->visitedCollections               =
         $this->eagerLoadingEntities             =
+        $this->eagerLoadingCollections          =
         $this->orphanRemovals                   = [];
 
         if ($this->evm->hasListeners(Events::onClear)) {
@@ -2352,6 +2358,10 @@ public function createEntity(string $className, array $data, array &$hints = [])
                 continue;
             }
 
+            if (! isset($hints['fetchMode'][$class->name][$field])) {
+                $hints['fetchMode'][$class->name][$field] = $assoc->fetch;
+            }
+
             $targetClass = $this->em->getClassMetadata($assoc->targetEntity);
 
             switch (true) {
@@ -2416,10 +2426,6 @@ public function createEntity(string $className, array $data, array &$hints = [])
                         break;
                     }
 
-                    if (! isset($hints['fetchMode'][$class->name][$field])) {
-                        $hints['fetchMode'][$class->name][$field] = $assoc->fetch;
-                    }
-
                     // Foreign key is set
                     // Check identity map first
                     // FIXME: Can break easily with composite keys if join column values are in
@@ -2514,9 +2520,13 @@ public function createEntity(string $className, array $data, array &$hints = [])
                     $reflField = $class->reflFields[$field];
                     $reflField->setValue($entity, $pColl);
 
-                    if ($assoc->fetch === ClassMetadata::FETCH_EAGER) {
-                        $this->loadCollection($pColl);
-                        $pColl->takeSnapshot();
+                    if ($hints['fetchMode'][$class->name][$field] === ClassMetadata::FETCH_EAGER) {
+                        if ($assoc->isOneToMany()) {
+                            $this->scheduleCollectionForBatchLoading($pColl, $class);
+                        } elseif ($assoc->isManyToMany()) {
+                            $this->loadCollection($pColl);
+                            $pColl->takeSnapshot();
+                        }
                     }
 
                     $this->originalEntityData[$oid][$field] = $pColl;
@@ -2532,7 +2542,7 @@ public function createEntity(string $className, array $data, array &$hints = [])
 
     public function triggerEagerLoads(): void
     {
-        if (! $this->eagerLoadingEntities) {
+        if (! $this->eagerLoadingEntities && ! $this->eagerLoadingCollections) {
             return;
         }
 
@@ -2545,11 +2555,69 @@ public function triggerEagerLoads(): void
                 continue;
             }
 
-            $class = $this->em->getClassMetadata($entityName);
+            $class   = $this->em->getClassMetadata($entityName);
+            $batches = array_chunk($ids, $this->em->getConfiguration()->getEagerFetchBatchSize());
 
-            $this->getEntityPersister($entityName)->loadAll(
-                array_combine($class->identifier, [array_values($ids)]),
-            );
+            foreach ($batches as $batchedIds) {
+                $this->getEntityPersister($entityName)->loadAll(
+                    array_combine($class->identifier, [$batchedIds]),
+                );
+            }
+        }
+
+        $eagerLoadingCollections       = $this->eagerLoadingCollections; // avoid recursion
+        $this->eagerLoadingCollections = [];
+
+        foreach ($eagerLoadingCollections as $group) {
+            $this->eagerLoadCollections($group['items'], $group['mapping']);
+        }
+    }
+
+    /**
+     * Load all data into the given collections, according to the specified mapping
+     *
+     * @param PersistentCollection[] $collections
+     */
+    private function eagerLoadCollections(array $collections, ToManyInverseSideMapping $mapping): void
+    {
+        $targetEntity = $mapping->targetEntity;
+        $class        = $this->em->getClassMetadata($mapping->sourceEntity);
+        $mappedBy     = $mapping->mappedBy;
+
+        $batches = array_chunk($collections, $this->em->getConfiguration()->getEagerFetchBatchSize(), true);
+
+        foreach ($batches as $collectionBatch) {
+            $entities = [];
+
+            foreach ($collectionBatch as $collection) {
+                $entities[] = $collection->getOwner();
+            }
+
+            $found = $this->getEntityPersister($targetEntity)->loadAll([$mappedBy => $entities]);
+
+            $targetClass    = $this->em->getClassMetadata($targetEntity);
+            $targetProperty = $targetClass->getReflectionProperty($mappedBy);
+            assert($targetProperty !== null);
+
+            foreach ($found as $targetValue) {
+                $sourceEntity = $targetProperty->getValue($targetValue);
+
+                $id     = $this->identifierFlattener->flattenIdentifier($class, $class->getIdentifierValues($sourceEntity));
+                $idHash = implode(' ', $id);
+
+                if ($mapping->indexBy !== null) {
+                    $indexByProperty = $targetClass->getReflectionProperty($mapping->indexBy);
+                    assert($indexByProperty !== null);
+                    $collectionBatch[$idHash]->hydrateSet($indexByProperty->getValue($targetValue), $targetValue);
+                } else {
+                    $collectionBatch[$idHash]->add($targetValue);
+                }
+            }
+        }
+
+        foreach ($collections as $association) {
+            $association->setInitialized(true);
+            $association->takeSnapshot();
         }
     }
 
@@ -2576,6 +2644,33 @@ public function loadCollection(PersistentCollection $collection): void
         $collection->setInitialized(true);
     }
 
+    /**
+     * Schedule this collection for batch loading at the end of the UnitOfWork
+     */
+    private function scheduleCollectionForBatchLoading(PersistentCollection $collection, ClassMetadata $sourceClass): void
+    {
+        $mapping = $collection->getMapping();
+        $name    = $mapping['sourceEntity'] . '#' . $mapping['fieldName'];
+
+        if (! isset($this->eagerLoadingCollections[$name])) {
+            $this->eagerLoadingCollections[$name] = [
+                'items'   => [],
+                'mapping' => $mapping,
+            ];
+        }
+
+        $owner = $collection->getOwner();
+        assert($owner !== null);
+
+        $id     = $this->identifierFlattener->flattenIdentifier(
+            $sourceClass,
+            $sourceClass->getIdentifierValues($owner),
+        );
+        $idHash = implode(' ', $id);
+
+        $this->eagerLoadingCollections[$name]['items'][$idHash] = $collection;
+    }
+
     /**
      * Gets the identity map of the UnitOfWork.
      *

psalm.xml

@@ -31,6 +31,10 @@
                 <referencedClass name="Doctrine\ORM\Exception\UnknownEntityNamespace"/>
                 <referencedClass name="Doctrine\ORM\Mapping\Driver\AnnotationDriver"/>
                 <referencedClass name="Doctrine\ORM\Mapping\Driver\YamlDriver"/>
+                <referencedClass name="Doctrine\ORM\Mapping\NamedNativeQueries"/>
+                <referencedClass name="Doctrine\ORM\Mapping\NamedNativeQuery"/>
+                <referencedClass name="Doctrine\ORM\Mapping\NamedQueries"/>
+                <referencedClass name="Doctrine\ORM\Mapping\NamedQuery"/>
                 <referencedClass name="Doctrine\ORM\Query\AST\InExpression"/>
                 <referencedClass name="Doctrine\ORM\Tools\Console\Command\ConvertDoctrine1SchemaCommand"/>
                 <referencedClass name="Doctrine\ORM\Tools\Console\Command\ConvertMappingCommand"/>