DDC-1639: Persister relies on primary key, not on the referencedColumnName #2061

Closed
opened 2026-01-22 13:39:08 +01:00 by admin · 7 comments
Owner

Originally created by @doctrinebot on GitHub (Feb 7, 2012).

Originally assigned to: @guilhermeblanco on GitHub.

Jira issue originally created by user zerkms:

Let's say we have entity

        class Location {
        ...
            /****
             * @ORM\ManyToOne(targetEntity="LocationType")
             * @ORM\JoinColumn(name="location*type*id", referencedColumnName="id")
             */
            private $locationType;
        ...
        }

(sorry, cannot format the code in several lines)

And another entity LocationType.

On save:

        $entity = new Location();

        $entity->setLocationType($lt);

        $em->persist($entity);
        $em->flush();

We get exception:

Undefined index: id in Doctrine/ORM/Persisters/BasicEntityPersister.php line 511

This happens because of Persister relies on the object's primary key, not on the specified referencedColumnName.

In my case I cannot make the LocationType.Id field a primary key because I implement temporal database schema

So the question is: if Persister relies always on PK - then referencedColumnName makes no sense, if referencedColumnName make any sense - then persister should respect its value

Originally created by @doctrinebot on GitHub (Feb 7, 2012). Originally assigned to: @guilhermeblanco on GitHub. Jira issue originally created by user zerkms: Let's say we have entity ``` class Location { ... /**** * @ORM\ManyToOne(targetEntity="LocationType") * @ORM\JoinColumn(name="location*type*id", referencedColumnName="id") */ private $locationType; ... } ``` (sorry, cannot format the code in several lines) And another entity `LocationType`. On save: ``` $entity = new Location(); $entity->setLocationType($lt); $em->persist($entity); $em->flush(); ``` We get exception: Undefined index: id in Doctrine/ORM/Persisters/BasicEntityPersister.php line 511 This happens because of Persister relies on the object's primary key, not on the specified referencedColumnName. In my case I cannot make the LocationType.Id field a primary key because I implement temporal database schema So the question is: if Persister relies always on PK - then referencedColumnName makes no sense, if referencedColumnName make any sense - then persister should respect its value
admin added the Bug label 2026-01-22 13:39:08 +01:00
admin closed this issue 2026-01-22 13:39:08 +01:00
Author
Owner

@doctrinebot commented on GitHub (Feb 10, 2012):

Comment created by @beberlei:

Formatted.

@doctrinebot commented on GitHub (Feb 10, 2012): Comment created by @beberlei: Formatted.
Author
Owner

@doctrinebot commented on GitHub (Feb 20, 2012):

Comment created by @beberlei:

referencedColumnName is necessary for two reasons:

  1. We don't want to recursively load all the metadata, so you have to duplicate some information so that we don't have to load them all into memory.
  2. You can have a composite key, in which case you have to exactly specify which column should point to which.

However the referencedColumnName always has to point to a primary key column.

The Doctrine\ORM\Tools\SchemaValidator class points you to this constraint as well.

@doctrinebot commented on GitHub (Feb 20, 2012): Comment created by @beberlei: referencedColumnName is necessary for two reasons: 1. We don't want to recursively load all the metadata, so you have to duplicate some information so that we don't have to load them all into memory. 2. You can have a composite key, in which case you have to exactly specify which column should point to which. However the referencedColumnName always has to point to a primary key column. The Doctrine\ORM\Tools\SchemaValidator class points you to this constraint as well.
Author
Owner

@doctrinebot commented on GitHub (Feb 20, 2012):

Comment created by zerkms:

Well, if it is _expected_ case - why the code throws undefined index?

Library should never throw any notices or warnings - the only correct way to interact with the client is exceptions, isn't it?

@doctrinebot commented on GitHub (Feb 20, 2012): Comment created by zerkms: Well, if it is **_expected**_ case - why the code throws `undefined index`? Library should never throw any notices or warnings - the only correct way to interact with the client is exceptions, isn't it?
Author
Owner

@doctrinebot commented on GitHub (Feb 20, 2012):

Comment created by @beberlei:

Because this sort of stuff is not validated at runtime, but only through the SchemaValidator tool. Runtime validation is too expensive for related metadata as it would require ALL metadata to be in memory, something we want to avoid.

@doctrinebot commented on GitHub (Feb 20, 2012): Comment created by @beberlei: Because this sort of stuff is not validated at runtime, but only through the SchemaValidator tool. Runtime validation is too expensive for related metadata as it would require ALL metadata to be in memory, something we want to avoid.
Author
Owner

@doctrinebot commented on GitHub (Feb 20, 2012):

Comment created by zerkms:

Uhm...

Is it that difficult to add isset() + throw new Exception for such cases?

So all developers see that it is not a bug, but an expected behaviour

What I want to say is that Doctrine2 shouldn't generate any notices, but throw exceptions. I didn't ask about runtime integrity checks (which is, indeed, expensive thing), but a simple isset() check that doctrine reads existent key (as long as an array comes from outside - the library cannot trust it and should always check if the key exists in the array).

@doctrinebot commented on GitHub (Feb 20, 2012): Comment created by zerkms: Uhm... Is it that difficult to add isset() + throw new Exception for such cases? So all developers see that it is not a bug, but an expected behaviour What I want to say is that Doctrine2 shouldn't generate any notices, but throw exceptions. I didn't ask about runtime integrity checks (which is, indeed, expensive thing), but a simple isset() check that doctrine reads existent key (as long as an array comes from outside - the library cannot trust it and should always check if the key exists in the array).
Author
Owner

@doctrinebot commented on GitHub (Feb 22, 2012):

Comment created by @guilhermeblanco:

Unfortunately, PHP does not perform well.
Even tough we agree with you that all libraries should not throw E_* errors, it's impossible for a project like Doctrine cover all attempts to protect developers.

In your specific case, suppose by adding this condition we include 0.01s overhead.
Such inclusion may be insignificant for a single execution, but BasicEntityPersister is called hundreds of times, depending of the amount of objects on your IdentityMap. This turns your simple condition of 0.01s into seconds.

Due to performance restrictions, we can't add this check, sorry.
We rely people to validate their schema to be able to deal correctly internally.

Closing the issue as "won't fix".

@doctrinebot commented on GitHub (Feb 22, 2012): Comment created by @guilhermeblanco: Unfortunately, PHP does not perform well. Even tough we agree with you that all libraries should not throw E_\* errors, it's impossible for a project like Doctrine cover all attempts to protect developers. In your specific case, suppose by adding this condition we include 0.01s overhead. Such inclusion may be insignificant for a single execution, but BasicEntityPersister is called hundreds of times, depending of the amount of objects on your IdentityMap. This turns your simple condition of 0.01s into seconds. Due to performance restrictions, we can't add this check, sorry. We rely people to validate their schema to be able to deal correctly internally. Closing the issue as "won't fix".
Author
Owner

@doctrinebot commented on GitHub (Feb 22, 2012):

Issue was closed with resolution "Won't Fix"

@doctrinebot commented on GitHub (Feb 22, 2012): Issue was closed with resolution "Won't Fix"
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#2061