[GH-1569] Optimize eager fetch for collections to batch query

[beberlei]

This supersedes and adjusts #1569 to current Doctrine 2.17 branch.

When you configure a one-to-many collection to be fetch=EAGER, then instead of immediately loading all collections it batches the fetch operation at the end by selecting all entities for all collections. Collections are loaded in batches of 100, configurable by Configuration::setEagerFetchBatchSize.

Changing the collection fetch mode can be done on a per query basis with $query->setFetchMode('rootEntity', 'association', ClassMetadata::FETCH_EAGER);. This can be done as an alternative to fetch join collection valued associations.

Open points:

• Naming: Its not really using subselect, alternative name could be "Batch". It supersede EAGER.
• requires removal of WITH keyword or prevention of WITH keyword when association is fetch=eager
• The batch loading o collections requires handling of maximum number of items in WHERE IN clause and potentially chunk it into multiple findBy operations.
• Should work across multiple layers, i.e. being recursive
• For fetch=eager the loading is delegated to persisters, here its going through the whole repository stack. Wondering if that must be improved code wise to go directly to persisters.
• There is no validation in ClassMetadataInfo if the selected fetch mode makes no sense for the entity.
• Does not work with ManyToMany, because they don't have a mapped field. Logic needs to be deferred to CollectionPersister.

Fixes #4762

[mpdude]

To summarize the suggested changes with my own words, this hooks into the place where entitites are being created during hydration. Thus, it sees all "new" entities, i. e. at the are first re-created from database values.

It tracks all those entities and their one-to-many (also many-to-many?) collections that are configured with fetch=SUBSELECT.

After the entities have been loaded, for every such collection a new query is performed through the target (referred-to) entity class' EntityRepository to find all of those entities by means the foreign key implementing the to-many association.

Then, loop over all those entities and add them to their respective collections to initialize those.

Not sure if indexBy/orderBy need to be taken into account.

[mpdude]

@beberlei Given your experience writing tricky solutions using subselects and doing AST manipulations like the Paginator component does, do you think it would be possible to write a helper component that could provide this feature and be used to do the "context sensitive optimization"? I also have the "multi-step hydration" explained at https://ocramius.github.io/blog/doctrine-orm-optimization-hydration/ in mind.

Idea along the lines of:

// Assume User has to-many association to Address in User::$addresses
$query = $entityManager->createQuery('SELECT u FROM ' . User::class . ' u WHERE ...');
$result = $query->getResult();

InitializeCollection::for($query, 'u.addresses');

What this would be supposed to do:

• Figure out the class that u refers to
• Clone and modify $query to SELECT u.id ... only
• Run a SELECT _tmp1, _tmp2 FROM {class of u} _tmp1 JOIN _tmp1.addresses _tmp2 WHERE _tmp1.id IN ({modified query})

Might be more tricky to get right with composite primary keys?

[JanTvrdik]

Disallow use of fetch=SUBSELECT on to-one associations.

Is there a technical reason to disallow this? Would it not still be benefical to fetch even has one associations in it's own query when the cardinality of the target entity is much lower to avoid fetching duplicated data?

[beberlei]

@JanTvrdik yes, but that is what fetch=EAGER does for to one already, either by doing a join, or by triggering all laods at once.

[greg0ire]

Thanks @beberlei !

[maxhelias]

Hi, I probably have a poor implementation or use of eager mode on my relationships but this exception creates a BC break on my side.
Maybe it should be replaced by a deprecation pointing to this ticket : #10978 to explain why this is a bad pratice ?

[aprat84]

Indeed this is a BC in v2... How was it not been cached before merging? :(

[krzysztof-ciszewski-tg]

What is the reason for this change?