Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.1
    • Component/s: None
    • Security Level: All
    • Labels:
      None

      Description

      I'm currently playing around with DDC-117. I came across a general
      problem which occurs with relations.

      Given the following two classes, should a
      @OneToOne/@OneToMany/@ManyToMany relation consider a composite ID or is
      that not planned with DDC-117?

      In the past, we assumed that an object is identified unique with an @Id
      column. However, if we support composite keys, an object needs to be
      identified by two or more columns, which we need to take into
      consideration when building queries and foreign keys. Is that correct?

      /**
       * @Entity
       */
      
      class Document {
      	/**
      	 * @Id
      	 * @Column(type="integer")
      	 * @GeneratedValue(strategy="AUTO")
      	 */
      	private $id;
      
      	/**
      	 * @Id
      	 * @Column(type="integer")
      	 */
      	private $version;
      
      	/**
      	 * @ManyToOne(targetEntity="Content")
      	 * Enter description here ...
      	 * @var unknown_type
      	 */
      	private $content;
      }
      
      /**
       * @Entity
       */
      class Content {
      	/**
      	 * @Id
      	 * @Column(type="integer")
      	 * @GeneratedValue(strategy="AUTO")
      	 */
      	private $id;
      
      	/**
      	 * @Id
      	 * @Column(type="integer")
      	 */
      	private $version;
      
      	/**
      	 * @ManyToOne(targetEntity="Document")
      	 */
      
      	private $document;
      }
      

      I know this might become a bit tricky, because $content refers to a
      single instance of Content, but we actually need two columns to identify
      it. That's how it looks if generated with Doctrine2 (DDC-117):

      mysql> describe Document;
      +------------+---------+------+-----+---------+-------+
      | Field      | Type    | Null | Key | Default | Extra |
      +------------+---------+------+-----+---------+-------+
      | id         | int(11) | NO   | PRI | NULL    |       |
      | version    | int(11) | NO   | PRI | NULL    |       |
      | content_id | int(11) | YES  | MUL | NULL    |       |
      +------------+---------+------+-----+---------+-------+
      
      mysql> describe Content;
      +-------------+---------+------+-----+---------+-------+
      | Field       | Type    | Null | Key | Default | Extra |
      +-------------+---------+------+-----+---------+-------+
      | id          | int(11) | NO   | PRI | NULL    |       |
      | version     | int(11) | NO   | PRI | NULL    |       |
      | document_id | int(11) | YES  | MUL | NULL    |       |
      +-------------+---------+------+-----+---------+-------+
      

      However, to make my example work, the tables would need to look the
      following:

      mysql> describe Content;
      +------------------+---------+------+-----+---------+-------+
      | Field            | Type    | Null | Key | Default | Extra |
      +------------------+---------+------+-----+---------+-------+
      | id               | int(11) | NO   | PRI | NULL    |       |
      | version          | int(11) | NO   | PRI | NULL    |       |
      | document_id      | int(11) | YES  | MUL | NULL    |       |
      | document_version | int(11) | YES  |     | NULL    |       |
      +------------------+---------+------+-----+---------+-------+
      
      mysql> describe Document;
      +-----------------+---------+------+-----+---------+-------+
      | Field           | Type    | Null | Key | Default | Extra |
      +-----------------+---------+------+-----+---------+-------+
      | id              | int(11) | NO   | PRI | NULL    |       |
      | version         | int(11) | NO   | PRI | NULL    |       |
      | content_id      | int(11) | YES  | MUL | NULL    |       |
      | content_version | int(11) | YES  |     | NULL    |       |
      +-----------------+---------+------+-----+---------+-------+
      

      It would be nice if we could discuss this one, as I feel it is important for an ORM.

        Activity

        Timo A. Hummel created issue -
        Benjamin Eberlei made changes -
        Field Original Value New Value
        Parent DDC-117 [ 10344 ]
        Issue Type New Feature [ 2 ] Sub-task [ 5 ]
        Hide
        Benjamin Eberlei added a comment -

        Hm i think this issue appears becaues "version" in your mapping is defined twice in the "Content" entity.

        You have rename the join column and it should work. Howevr ClassMetadata should throw an exception.

        Show
        Benjamin Eberlei added a comment - Hm i think this issue appears becaues "version" in your mapping is defined twice in the "Content" entity. You have rename the join column and it should work. Howevr ClassMetadata should throw an exception.
        Hide
        Timo A. Hummel added a comment -

        So in theory, doctrine should handle my example with DDC-117? Will try that out later with unique field names and report back. If that would work already, this would be amazing!

        Show
        Timo A. Hummel added a comment - So in theory, doctrine should handle my example with DDC-117 ? Will try that out later with unique field names and report back. If that would work already, this would be amazing!
        Hide
        Benjamin Eberlei added a comment -

        btw your mapping is somewhat wrong. you cannot have two @ManyToOne that connect the same two entities with each other. One has to be @OneToMany

        Show
        Benjamin Eberlei added a comment - btw your mapping is somewhat wrong. you cannot have two @ManyToOne that connect the same two entities with each other. One has to be @OneToMany
        Hide
        Timo A. Hummel added a comment -

        Benjamin, I finally had time to check out more things. In fact, Doctrine with DDC-117 completely ignores multiple foreign keys.

        Even if I replace @ManyToOne with @OneToMany, and version with fversion in Content, Doctrine still only creates a reference (and columns!) with content_id only. I would have expected that that I find at least content_id and content_version within the Document table

        I also tried to specify mappedBy="document,version", but this also didn't work.

        So again the question: Is my initial example meant to be possible with DDC-117, or is it not?

        Show
        Timo A. Hummel added a comment - Benjamin, I finally had time to check out more things. In fact, Doctrine with DDC-117 completely ignores multiple foreign keys. Even if I replace @ManyToOne with @OneToMany, and version with fversion in Content, Doctrine still only creates a reference (and columns!) with content_id only. I would have expected that that I find at least content_id and content_version within the Document table I also tried to specify mappedBy="document,version", but this also didn't work. So again the question: Is my initial example meant to be possible with DDC-117 , or is it not?
        Hide
        Timo A. Hummel added a comment - - edited

        I just created a better test.

        User.php
        <?php
        /**
         * @Entity
         */
        class User {
        	/**
        	 * @Id
        	 * @Column(type="integer")
        	 * @GeneratedValue(strategy="AUTO")
        	 */
        	private $id;
        	
        	/**
        	 * @Column(type="string")
        	 */
        	private $name;
        	
        	/**
        	 * @OneToMany(targetEntity="PhoneNumber",mappedBy="id")
        	 */
        	private $phoneNumbers;
        }
        
        PhoneNumber.php
        <?php
        /**
         * @Entity
         */
        class PhoneNumber {
        	/**
        	 * @Id
        	 * @Column(type="integer")
        	 */
        	private $id;
        	
        	/**
        	 * @Id
        	 * @ManyToOne(targetEntity="User",cascade={"all"})
        	 */
        	private $user;
        	
        	/**
        	 * @Column(type="string")
        	 */
        	private $phonenumber;
        	
        	public function setId ($id) {
        		$this->id = $id;
        	}
        	
        	public function setUser (User $user) {
        		$this->user = $user;
        	}
        	
        	public function setPhoneNumber ($phoneNumber) {
        		$this->phonenumber = $phoneNumber;
        	}
        }
        
        PhoneCall.php
        <?php 
        /**
         * @Entity
         */
        class PhoneCall {
        	/**
        	 * @Id
        	 * @Column(type="integer")
        	 * @GeneratedValue(strategy="AUTO")
        	 */
        	private $id;
        	
        	/**
        	 * @OneToOne(targetEntity="PhoneNumber",cascade={"all"})
        	 */
        	private $phonenumber;
        	
        	/**
        	 * @Column(type="string",nullable=true)
        	 */
        	private $callDate;
        	
        	public function setPhoneNumber (PhoneNumber $phoneNumber) {
        		$this->phonenumber = $phoneNumber;
        	}
        	
        }
        

        This is a very basic mapping example, where one user may have many phone numbers. Also we have a PhoneCall entity which stores the call made. Since a single phone number isn't just identified by id, but also by user_id, Doctrine should create two fields in PhoneCall for that relation: phonenumber_id and phonenumber_user_id, so that we can retrieve the PhoneNumber object by PhoneCall's phonenumber property.

        If you agree that my example should be working, I'm willing to write a test case and see if I can contribute code to make that happen.

        Show
        Timo A. Hummel added a comment - - edited I just created a better test. User.php <?php /** * @Entity */ class User { /** * @Id * @Column(type= "integer" ) * @GeneratedValue(strategy= "AUTO" ) */ private $id; /** * @Column(type= "string" ) */ private $name; /** * @OneToMany(targetEntity= "PhoneNumber" ,mappedBy= "id" ) */ private $phoneNumbers; } PhoneNumber.php <?php /** * @Entity */ class PhoneNumber { /** * @Id * @Column(type= "integer" ) */ private $id; /** * @Id * @ManyToOne(targetEntity= "User" ,cascade={ "all" }) */ private $user; /** * @Column(type= "string" ) */ private $phonenumber; public function setId ($id) { $ this ->id = $id; } public function setUser (User $user) { $ this ->user = $user; } public function setPhoneNumber ($phoneNumber) { $ this ->phonenumber = $phoneNumber; } } PhoneCall.php <?php /** * @Entity */ class PhoneCall { /** * @Id * @Column(type= "integer" ) * @GeneratedValue(strategy= "AUTO" ) */ private $id; /** * @OneToOne(targetEntity= "PhoneNumber" ,cascade={ "all" }) */ private $phonenumber; /** * @Column(type= "string" ,nullable= true ) */ private $callDate; public function setPhoneNumber (PhoneNumber $phoneNumber) { $ this ->phonenumber = $phoneNumber; } } This is a very basic mapping example, where one user may have many phone numbers. Also we have a PhoneCall entity which stores the call made. Since a single phone number isn't just identified by id, but also by user_id, Doctrine should create two fields in PhoneCall for that relation: phonenumber_id and phonenumber_user_id, so that we can retrieve the PhoneNumber object by PhoneCall's phonenumber property. If you agree that my example should be working, I'm willing to write a test case and see if I can contribute code to make that happen.
        Hide
        Timo A. Hummel added a comment -

        I'm preparing a real-world test script; no need to respond for now.

        Show
        Timo A. Hummel added a comment - I'm preparing a real-world test script; no need to respond for now.
        Hide
        Timo A. Hummel added a comment - - edited

        As promised, here's the real-world test script. Note that I adjusted the code for the example entities due to DDC-891.

        Test Script
        /* Create two test users: albert and alfons */
        $albert = new User;
        $albert->setName("albert");
        $em->persist($albert);
        
        $alfons = new User;
        $alfons->setName("alfons");
        $em->persist($alfons);
        
        /* Assign two phone numbers to each user */
        $phoneAlbert1 = new PhoneNumber();
        $phoneAlbert1->setUser($albert);
        $phoneAlbert1->setId(1);
        $phoneAlbert1->setPhoneNumber("albert home: 012345");
        $em->persist($phoneAlbert1);
        
        $phoneAlbert2 = new PhoneNumber();
        $phoneAlbert2->setUser($albert);
        $phoneAlbert2->setId(2);
        $phoneAlbert2->setPhoneNumber("albert mobile: 67890");
        $em->persist($phoneAlbert2);
        
        $phoneAlfons1 = new PhoneNumber();
        $phoneAlfons1->setId(1);
        $phoneAlfons1->setUser($alfons);
        $phoneAlfons1->setPhoneNumber("alfons home: 012345");
        $em->persist($phoneAlfons1);
        
        $phoneAlfons2 = new PhoneNumber();
        $phoneAlfons2->setId(2);
        $phoneAlfons2->setUser($alfons);
        $phoneAlfons2->setPhoneNumber("alfons mobile: 67890");
        $em->persist($phoneAlfons2);
        
        /* We call alfons and albert once on their mobile numbers */
        $call1 = new PhoneCall();
        $call1->setPhoneNumber($phoneAlfons2);
        $em->persist($call1);
        
        $call2 = new PhoneCall();
        $call2->setPhoneNumber($phoneAlbert2);
        $em->persist($call2);
        
        $em->flush();
        

        During the flush, Doctrine fails with

        Integrity constraint violation: 1062 Duplicate entry '2' for key 'PhoneCall_phonenumber_id_uniq

        because Doctrine does not create the composite primary key (which is id and user_id) as foreign key (Doctrine only adds id).

        Show
        Timo A. Hummel added a comment - - edited As promised, here's the real-world test script. Note that I adjusted the code for the example entities due to DDC-891 . Test Script /* Create two test users: albert and alfons */ $albert = new User; $albert->setName( "albert" ); $em->persist($albert); $alfons = new User; $alfons->setName( "alfons" ); $em->persist($alfons); /* Assign two phone numbers to each user */ $phoneAlbert1 = new PhoneNumber(); $phoneAlbert1->setUser($albert); $phoneAlbert1->setId(1); $phoneAlbert1->setPhoneNumber( "albert home: 012345" ); $em->persist($phoneAlbert1); $phoneAlbert2 = new PhoneNumber(); $phoneAlbert2->setUser($albert); $phoneAlbert2->setId(2); $phoneAlbert2->setPhoneNumber( "albert mobile: 67890" ); $em->persist($phoneAlbert2); $phoneAlfons1 = new PhoneNumber(); $phoneAlfons1->setId(1); $phoneAlfons1->setUser($alfons); $phoneAlfons1->setPhoneNumber( "alfons home: 012345" ); $em->persist($phoneAlfons1); $phoneAlfons2 = new PhoneNumber(); $phoneAlfons2->setId(2); $phoneAlfons2->setUser($alfons); $phoneAlfons2->setPhoneNumber( "alfons mobile: 67890" ); $em->persist($phoneAlfons2); /* We call alfons and albert once on their mobile numbers */ $call1 = new PhoneCall(); $call1->setPhoneNumber($phoneAlfons2); $em->persist($call1); $call2 = new PhoneCall(); $call2->setPhoneNumber($phoneAlbert2); $em->persist($call2); $em->flush(); During the flush, Doctrine fails with Integrity constraint violation: 1062 Duplicate entry '2' for key 'PhoneCall_phonenumber_id_uniq because Doctrine does not create the composite primary key (which is id and user_id) as foreign key (Doctrine only adds id).
        Hide
        Benjamin Eberlei added a comment -

        The problem here is your @OneToOne relation. It implicitly sets a unique index. The primary key generated by your mapping is ok.

        Try to modify your code stating @OneToOne(unique=false)

        Additionally you have to specifiy the join columns. You are using the default, which obviously doesn't work for a composite approach (that is not default):

        /**
         * @Entity
         */
        class DDC881PhoneCall
        {
        
            /**
             * @Id
             * @Column(type="integer")
             * @GeneratedValue(strategy="AUTO")
             */
            private $id;
            /**
             * @OneToOne(targetEntity="DDC881PhoneNumber",cascade={"all"})
             * @JoinColumns({
             *  @JoinColumn(name="phonenumber_id", referencedColumnName="id"),
             *  @JoinColumn(name="user_id", referencedColumnName="user_id")
             * })
             */
            private $phonenumber;
            /**
             * @Column(type="string",nullable=true)
             */
            private $callDate;
        
            public function setPhoneNumber(DDC881PhoneNumber $phoneNumber)
            {
                $this->phonenumber = $phoneNumber;
            }
        
        }
        

        That would be the right approach if there were not a bug in SchemaTool i need to fix for this to work

        Show
        Benjamin Eberlei added a comment - The problem here is your @OneToOne relation. It implicitly sets a unique index. The primary key generated by your mapping is ok. Try to modify your code stating @OneToOne(unique=false) Additionally you have to specifiy the join columns. You are using the default, which obviously doesn't work for a composite approach (that is not default): /** * @Entity */ class DDC881PhoneCall { /** * @Id * @Column(type= "integer" ) * @GeneratedValue(strategy= "AUTO" ) */ private $id; /** * @OneToOne(targetEntity= "DDC881PhoneNumber" ,cascade={ "all" }) * @JoinColumns({ * @JoinColumn(name= "phonenumber_id" , referencedColumnName= "id" ), * @JoinColumn(name= "user_id" , referencedColumnName= "user_id" ) * }) */ private $phonenumber; /** * @Column(type= "string" ,nullable= true ) */ private $callDate; public function setPhoneNumber(DDC881PhoneNumber $phoneNumber) { $ this ->phonenumber = $phoneNumber; } } That would be the right approach if there were not a bug in SchemaTool i need to fix for this to work
        Hide
        Benjamin Eberlei added a comment -

        This is a really tricky issue, we need to think about it a little longer.

        Show
        Benjamin Eberlei added a comment - This is a really tricky issue, we need to think about it a little longer.
        Hide
        Benjamin Eberlei added a comment -

        Fixed.

        Show
        Benjamin Eberlei added a comment - Fixed.
        Benjamin Eberlei made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 2.1 [ 10022 ]
        Resolution Fixed [ 1 ]
        Benjamin Eberlei made changes -
        Workflow jira [ 12121 ] jira-feedback [ 14647 ]
        Benjamin Eberlei made changes -
        Workflow jira-feedback [ 14647 ] jira-feedback2 [ 16511 ]
        Benjamin Eberlei made changes -
        Workflow jira-feedback2 [ 16511 ] jira-feedback3 [ 18764 ]

        This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

        • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DDC-881, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            Roman S. Borschel
            Reporter:
            Timo A. Hummel
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: