Doctrine 2 - ORM

DDC-117: Linked Objects with composite key

Details

  • Type: Sub-task Sub-task
  • Status: Resolved 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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
Benjamin Eberlei added a comment -

Fixed.

Show
Benjamin Eberlei added a comment - Fixed.

People

  • Assignee:
    Roman S. Borschel
    Reporter:
    Timo A. Hummel
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: