Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix extra lazy get #710

Merged
merged 2 commits into from
Jun 30, 2013
Merged

Conversation

sandermarechal
Copy link
Contributor

I made a big mistake in PR #706, my apologies. The get() function in the OneToManyPersister and ManyToManyPersister did not add the collection owner to the load query. The unit tests failed to detect this because the first entity is always used (ID 1) was used to test with.

The first commit in this PR fixes this for the OneToMayPersister.

I could not find a way to fix this for the ManyToManyPersister. Problem is that you can only use conditions on the owning side of a ManyToMany relation, not on the inverse side. Code like load(array($mapping['inversedBy'] => $coll->getOwner()), ...) gave an ORMException.

Therefor, the second commit in this PR removes the extra-lazy-get for ManyToMany relations. If anyone has any ideas how to make this work for the ManyToMany side, please let me know.

@stof
Copy link
Member

stof commented Jun 27, 2013

couldn't we keep it for ManyToMany on the working side of the relation (and fallbacking to the initialization for the other side) ?

@beberlei
Copy link
Member

@stof i am +1 for this solution

@sandermarechal
Copy link
Contributor Author

@stof I tried that initially, but I couldn't make it work. I rewrote the testGetIndexByManyToMany to use group->users instead of user->groups and added the mappedBy condition in the ManyToManyPersister, but I got errors that a joinColumn index didn't exist from somewhere else in the ORM.

I'm happy to push a branch with that code to my github fork. Perhaps one of you can help me figure out what the problem was with it. Unfortunately I won't be able to do so until Monday.

@guilhermeblanco
Copy link
Member

PR #1249 addresses both owning and inverse side extra lazy get on many to many associations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants