Cascade detach should detach lazy collections too

[goetas]

This is a follow up of #10065

When a collection is "LAZY", it is not being detached when detach is called.

In order to detach all the entities, the collection needs to be initialized (similarly to what is done for remove()).

To give a more real example:

Given this class:

class User
{
    /**
     * @ORM\OneToMany(targetEntity="Address", cascade={"detach"}, fetch="LAZY")
     */
    public $addresses;
}

before my change

$user = $em->find(User::class, $user->id);
$em->detach($user);

$adr1 = $user->addresses[0]; 
// this will trigger an SQL query to fin the address 
// but it should not happen because the user is detached, addresses should be detached as well

$m->contains($adr1); // is managed ... but it should have been detached

after my change

$user = $em->find(User::class, $user->id);
$em->detach($user);

$adr1 = $user->addresses[0];  // no SQL triggered at this point

$m->contains($adr1); // not managed

[goetas]

I had to make this changes because "child" objects should be detached first, and then the parents (otherwise the IDs are not available for some DBs as sqlite)

[beberlei]

I am afraid this is going to break some people event listeners :)

We should add this the UPGRADE notes to warn people. We don't guarantee the order so its not a BC break, but a heads up is always nice.

[SenseException]

Your example is describing that there shouldn't be an SQL query triggered for addresses when detached. Shouldn't the test make sure that this is the case?

$adr1 = $user->addresses[0];  // no SQL triggered at this point

When I run $this->getQueryLog() after $ad = $user->addresses[0], I find the following SQL query in the logger:

SELECT t0.id AS id_1, t0.data AS data_2, t0.user_id AS user_id_3 FROM LazyEagerCollectionAddress t0 WHERE t0.user_id = ?

If I understood the PR correctly we have two goals: detach the lazy stuff and prevent an SQL query for the lazy objects. Am I understanding it correctly that this query shouldn't happen in the test?

[beberlei]

You found a tricky problem, thanks for that!

but we should improve the implementation to be more performance sensitive imho, wdyt?

[beberlei]

I am afraid this is going to break some people event listeners :)

We should add this the UPGRADE notes to warn people. We don't guarantee the order so its not a BC break, but a heads up is always nice.

[beberlei]

This should be documented in the docs, because it could lead to the operation becoming very expensive.

It should also be documented in upgrade notes.

You could also implement a shortcut here, if no entity of $assoc entity is in the identity map, then loading the collection from database will not yield any benefit.

For both lazy and extra lazy you could also make a query to load only the IDs, then check if they are in the identity map to detech. This would make the query + loading cheaper. The reaosn is that the entities loaded here are immediately going out of scope and garbage collected anyways.