[DDC-3346] Failing test for issue (bad findOneBy behaviour with eager fetch)

[scaytrase]

Here is the test for DDC-3346 issue

[doctrinebot]

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3531

We use Jira to track the state of pull requests and the versions they got
included in.

[Ocramius]

Can you remove these fields?

[Ocramius]

Wondering about the resolution path for this: my opinion is that we should just disable the eager loading when a LIMIT is provided. Thoughts?

[scaytrase]

@Ocramius I've updated test. Does it looks right?

Generally, I see three options:

1. Disable eager loading. This leads to unwanted additional queries (one per each entity association)
2. Fetch entity again if found (do something like $entity = $this->find($entity->getId()) internally). This options will make only one additional query constantly.
3. Make more complex query.

[Ocramius]

Disable eager loading.

This is what I'll try doing. The additional queries are a side-effect that cannot be avoided.

Fetch entity again if found

Not really possible, as you'll just hit the identity map in this case. I'll look into it though.

Make more complex query

This involves going through the paginator API (2 queries), and we really don't want to do that given the horrible amount of paginator-related issues with mssql and other exotic DBMS.

[scaytrase]

I took a look through the sources, and the problem really occurs with any limit set. I've updated the test to respect this.

[scaytrase]

Also I realized that offset parameter in findOneBy \ findBy will also should lead to failure.

I think that disabling eager loading is dirty workaround, because it would have unattended performance impact.
Third way is more clean and correct IMO, but as far as I can understand you, it is rather hard to implement (I almost don't know what is paginator API).

Second way is something intermediate.

[Ocramius]

I think that disabling eager loading is dirty workaround, because it would have unattended performance impact.

Doing the opposite is actually worse, as the state of the loaded entities will be corrupted, and all event listeners will be broken instead. Patching state by refreshing entities is not a good idea.

If the user configures eager loads, it's his own fault there: a second query is an acceptable tradeoff.

[Ocramius]

I have a fix for this, but it's not complete yet. Fix will be applied in 2.5: 2.4.x will probably get an exception message instead (for now).

[Ocramius]

This bug does not affect 2.4.x, as the eager loading does not work with TO_MANY associations