Doctrine 2 - ORM

Possible issue with PersistentCollection#getDelete/InsertDiff()

Details

  • Type: Improvement Improvement
  • Status: Reopened Reopened
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 2.1
  • Fix Version/s: 2.4
  • Component/s: None
  • Security Level: All
  • Labels:
    None

Description

Using the following code, when you go from (1, 2) to (1), (2) is deleted as expected. However, if you go from (1, 2) to (2), (1) and (2) are deleted and (2) is then inserted. Is this the desired behaviour? (i.e. 2 extra queries)

$bracket->getTournamentLocations()->takeSnapshot();

$col = $bracket->getTournamentLocations()->unwrap();

$col->clear();

foreach ($form->getValue('tournamentLocations') as $id) {
    $col->add($em->getReference('Tournaments_Model_TournamentLocation', $id));
}

$bracket->getTournamentLocations()->setDirty(true);

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Benjamin Eberlei added a comment -

First, you are using internal API therefore you are on your own anyways.

This is marked as improvment now, the functionality works, it may just be inefficient.

Show
Benjamin Eberlei added a comment - First, you are using internal API therefore you are on your own anyways. This is marked as improvment now, the functionality works, it may just be inefficient.
Hide
Permalink
Guilherme Blanco added a comment -

Hi,

I'm marking issue as invalid because you're conceptually wrong.
What you're trying to do is telling that a collection of new entities is holded by a collection of Persistent entities.
The reference internally of PersistentCollection to ArrayCollection means a lot here.

Correct code would be you to regenerate the collection (a new ArrayCollection) and just assign it to setTournamentLocations($newCollection);

Does this explanation is enough for you?

Cheers,

Show
Guilherme Blanco added a comment - Hi, I'm marking issue as invalid because you're conceptually wrong. What you're trying to do is telling that a collection of new entities is holded by a collection of Persistent entities. The reference internally of PersistentCollection to ArrayCollection means a lot here. Correct code would be you to regenerate the collection (a new ArrayCollection) and just assign it to setTournamentLocations($newCollection); Does this explanation is enough for you? Cheers,
Hide
Permalink
Glen Ainscow added a comment -

Hi Guilherme,

If I do this:

$locations = new ArrayCollection();

foreach ($form->getValue('tournamentLocations') as $id) {
    $locations->add($em->getReference('Tournaments_Model_TournamentLocation', $id));
}

$bracket->setTournamentLocations($locations);

... then all the records are deleted, before adding the new records. This is inefficient and causes extra, unnecessary write operations.

Can't Doctrine perform diffs when persisting the collection, so that only the necessary deletes and inserts are executed?

Show
Glen Ainscow added a comment - Hi Guilherme, If I do this: $locations = new ArrayCollection(); foreach ($form->getValue('tournamentLocations') as $id) { $locations->add($em->getReference('Tournaments_Model_TournamentLocation', $id)); } $bracket->setTournamentLocations($locations); ... then all the records are deleted, before adding the new records. This is inefficient and causes extra, unnecessary write operations. Can't Doctrine perform diffs when persisting the collection, so that only the necessary deletes and inserts are executed?
Hide
Permalink
Guilherme Blanco added a comment -

We could add it, but I don't think it worth the effort.
Main problem with this one is that we use C-level binary comparison to get the diff. That's what you entities/hash pointers are different.
We would have to write our own diff-comparator for both collections, which would probably slowdown the entire Doctrine.

I'd rather consider that it's not possible to be done at the moment, but I need much more investigation for that. This will be something that I'll probably only do when I look at this issue again with a lot of time (which is really hard to happen).

If you have some spare time, feel free to make some attempts.
Just don't forget to enable performance tests in Doctrine Unit Test suite.

Show
Guilherme Blanco added a comment - We could add it, but I don't think it worth the effort. Main problem with this one is that we use C-level binary comparison to get the diff. That's what you entities/hash pointers are different. We would have to write our own diff-comparator for both collections, which would probably slowdown the entire Doctrine. I'd rather consider that it's not possible to be done at the moment, but I need much more investigation for that. This will be something that I'll probably only do when I look at this issue again with a lot of time (which is really hard to happen). If you have some spare time, feel free to make some attempts. Just don't forget to enable performance tests in Doctrine Unit Test suite.

People

  • Assignee:
    Guilherme Blanco
    Reporter:
    Glen Ainscow
Vote (0)
Watch (1)

Dates

  • Created:
    Updated: