Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-1283

Possible issue with PersistentCollection#getDelete/InsertDiff()

    Details

    • Type: Improvement Improvement
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.1
    • Fix Version/s: 2.5
    • 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

        Hide
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated: