Fixed typo in hints. Caused slow loading of eager entities. #611

Merged
merged 3 commits into from Apr 14, 2013

Projects

None yet

4 participants

@stefankleff

This small typo prevents entities from being fetched with the deferred- eager-loading path.

@doctrinebot
Collaborator

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DDC-2346

@Ocramius
Member

Hey @stefankleff! Good catch, but do you have a test case for this fix? (to avoid this in future)

@stefankleff

This already took me almost two days. It would be nice if someone else can add some tests 0:-)

@Ocramius
Member

@stefankleff do you have a query to start from? I can do it if I know what kind of issue spawned this.

@stefankleff

I ran into that while dealing with a rather complex use case. I think it should be sufficient to do the following: A has a bidirectional eager 1:n relation r to B. fetchAll(A) should hydrate the objects with 2 queries not with 1 + n

@stefankleff

Important: You have to use the "OOP-way" (with repository) not the Query or QueryBuilder.

@Ocramius
Member

I see. Will check if a test is possible :)

@beberlei
Member

Can you make a constant out of the "magic string"?

@Ocramius Ocramius added a commit to Ocramius/doctrine2 that referenced this pull request Mar 12, 2013
@Ocramius Ocramius Adding failing test for DDC-2346 doctrine/doctrine2#611 e468ced
@Ocramius
Member

@stefankleff please check stefankleff/doctrine2#1 (quick and dirty one, but I couldn't reproduce the fix)

@stefankleff

First of all thanks for your test! The use case I described yesterday was the wrong way: fetch all Bs where every B has a eager to-one relation to A. Doing this will result in a regular join. But if C extends B and you fetch all Cs the JoinedSubclassPersister is used, which is not able to generate the SQL and the resultset for the join. Therefore the "join" is done in the UoW with the help of deferred loading. Without the fix this fails and every related A is fetched while creating the C entitity - not afterwards.

@Ocramius
Member

@stefankleff aha, didn't know there was an inheritance :) Thank you for incorporating the test!

@stefankleff

And a constant would be much better - but I'm not sure in which class it should be defined. I would prefer the UoW. Any suggestions?

@beberlei
Member

Adding the constant in the UnitOfWork is good.

@beberlei
Member
beberlei commented Apr 4, 2013

@stefankleff any news?

@beberlei beberlei merged commit 52b2e06 into doctrine:master Apr 14, 2013

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment