Skip to content

Commit

Permalink
Merge branch '2.17.x' into 3.0.x
Browse files Browse the repository at this point in the history
* 2.17.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.
  [doctrineGH-1569] Add new SUBSELECT fetch mode for OneToMany associations.
  • Loading branch information
derrabus committed Nov 15, 2023
2 parents 21466a0 + 1267f48 commit 56df970
Show file tree
Hide file tree
Showing 12 changed files with 299 additions and 19 deletions.
9 changes: 9 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions docs/en/reference/working-with-objects.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~~~~~~~~~~~

Expand Down
10 changes: 10 additions & 0 deletions lib/Doctrine/ORM/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,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;
Expand Down
8 changes: 8 additions & 0 deletions lib/Doctrine/ORM/Query/QueryException.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
Expand Down
4 changes: 3 additions & 1 deletion lib/Doctrine/ORM/Query/SqlWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,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.
Expand Down
119 changes: 107 additions & 12 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -292,6 +294,9 @@ class UnitOfWork implements PropertyChangedListener
*/
private array $eagerLoadingEntities = [];

/** @var array<string, array<string, mixed>> */
private array $eagerLoadingCollections = [];

protected bool $hasCache = false;

/**
Expand Down Expand Up @@ -2218,6 +2223,7 @@ public function clear(): void
$this->pendingCollectionElementRemovals =
$this->visitedCollections =
$this->eagerLoadingEntities =
$this->eagerLoadingCollections =
$this->orphanRemovals = [];

if ($this->evm->hasListeners(Events::onClear)) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand All @@ -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();
}
}

Expand All @@ -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.
*
Expand Down
4 changes: 4 additions & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand Down
Loading

0 comments on commit 56df970

Please sign in to comment.