Allow @Id on @ManyToOne fields (DDC-117)

[DDC-881] DDC-117: Linked Objects with composite key Created: 18/Nov/10  Updated: 02/Jan/11  Resolved: 02/Jan/11

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: None
Affects Version/s: None
Fix Version/s: 2.1
Security Level: All

Type: Sub-task Priority: Minor
Reporter: Timo A. Hummel Assignee: Roman S. Borschel
Resolution: Fixed Votes: 0
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.



 Comments   
Comment by Benjamin Eberlei [ 18/Nov/10 ]

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.

Comment by Timo A. Hummel [ 18/Nov/10 ]

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!

Comment by Benjamin Eberlei [ 18/Nov/10 ]

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

Comment by Timo A. Hummel [ 24/Nov/10 ]

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?

Comment by Timo A. Hummel [ 24/Nov/10 ]

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.

Comment by Timo A. Hummel [ 24/Nov/10 ]

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

Comment by Timo A. Hummel [ 25/Nov/10 ]

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

Comment by Benjamin Eberlei [ 28/Dec/10 ]

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

Comment by Benjamin Eberlei [ 29/Dec/10 ]

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

Comment by Benjamin Eberlei [ 02/Jan/11 ]

Fixed.

Generated at Thu Oct 30 14:10:20 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.