DDC-881: DDC-117: Linked Objects with composite key #5405

Closed
doctrinebot opened this Issue Nov 18, 2010 · 11 comments

1 participant

@doctrinebot

Jira issue originally created by user felicitus:

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;
<ins>------------</ins>---------<ins>------</ins>-----<ins>---------</ins>-------<ins>
| Field      | Type    | Null | Key | Default | Extra |
</ins>------------<ins>---------</ins>------<ins>-----</ins>---------<ins>-------</ins>
| id         | int(11) | NO   | PRI | NULL    |       |
| version    | int(11) | NO   | PRI | NULL    |       |
| content_id | int(11) | YES  | MUL | NULL    |       |
<ins>------------</ins>---------<ins>------</ins>-----<ins>---------</ins>-------<ins>

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;
<ins>------------------</ins>---------<ins>------</ins>-----<ins>---------</ins>-------<ins>
| Field            | Type    | Null | Key | Default | Extra |
</ins>------------------<ins>---------</ins>------<ins>-----</ins>---------<ins>-------</ins>
| 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    |       |
<ins>------------------</ins>---------<ins>------</ins>-----<ins>---------</ins>-------<ins>
mysql> describe Document;
</ins>-----------------<ins>---------</ins>------<ins>-----</ins>---------<ins>-------</ins>
| Field           | Type    | Null | Key | Default | Extra |
<ins>-----------------</ins>---------<ins>------</ins>-----<ins>---------</ins>-------<ins>
| 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    |       |
</ins>-----------------<ins>---------</ins>------<ins>-----</ins>---------<ins>-------</ins>

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

@doctrinebot

Comment created by @beberlei:

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.

@doctrinebot

Comment created by felicitus:

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!

@doctrinebot

Comment created by @beberlei:

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

@doctrinebot

Comment created by felicitus:

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 contentid only. I would have expected that that I find at least content_id and contentversion 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?

@doctrinebot

Comment created by felicitus:

I just created a better test.

<?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;
}
<?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;
    }
}
<?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 userid, Doctrine should create two fields in PhoneCall for that relation: phonenumber_id and phonenumber_userid, 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.

@doctrinebot

Comment created by felicitus:

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

@doctrinebot

Comment created by felicitus:

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

/** 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).

@doctrinebot

Comment created by @beberlei:

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 :-)

@doctrinebot

Comment created by @beberlei:

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

@doctrinebot

Comment created by @beberlei:

Fixed.

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot doctrinebot added this to the 2.1 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment