Avoid PersistentCollection::isEmpty() to fully load the collection. #912
Conversation
|
Hello, thank you for creating this pull request. I have automatically opened an issue http://www.doctrine-project.org/jira/browse/DDC-2921 We use Jira to track the state of pull requests and the versions they got |
|
@BenMorel needs a test :-) |
|
@Ocramius I thought this would be covered by existing tests TBH, as it's not changing existing functionality. |
|
@BenMorel future changes shall not re-introduce the additional query ;) |
|
True, I understand now ;-) |
|
@BenMorel ping? |
|
@Ocramius I didn't find the time yet, will try to do this over the week-end! |
|
Thanks! |
|
@Ocramius I added a functional test to ensure that the collection is not initialized after calling Also, fixed a wrong documentation type ( |
| $this->initialize(); | ||
|
|
||
| return $this->coll->isEmpty(); | ||
| return $this->count() == 0; |
There was a problem hiding this comment.
I think this could be even more improved, by adding a check whether any NEW entities are present (which would avoid count() thus save sql query).
There was a problem hiding this comment.
Not sure to understand what you mean, Michael: when you load an entity and one property is a PersistentCollection, you can't know whether it contains any entity before doing an SQL query, even if no NEW entities have been added to it?
There was a problem hiding this comment.
@BenMorel it would probably just be the following:
public function isEmpty()
{
return $this->coll->isEmpty() && $this->count() === 0;
}That would indeed save 1 query if a PersistentCollection#add() happened before calling PersistentCollection#count()
There was a problem hiding this comment.
Oh, I get it now. Good point. Let me have a look at that.
There was a problem hiding this comment.
Great then, thanks for the suggestion, and thanks @beberlei for merging at the speed of light!
… extra lazy fetch.
|
This is a genious change :) very nice |
Avoid PersistentCollection::isEmpty() to fully load the collection.
Extra lazy support for it was added a long time ago (see doctrine#912) but was never properly documented.
Extra lazy support for it was added a long time ago (see doctrine#912) but was never properly documented.
This is a suggested simple performance improvement for
PersistentCollection.At the moment,
isEmpty()always fully loads the collection. By changing the code to rely oncount(), which makes advantage of extra lazy fetch when the collection is not loaded, this saves the overhead of loading the entire collection when one just wants to check whether the collection is empty.