Set JoinColumn::referencedColumnName based on the target entity instead of id #6940

New Issue

admin · 2026-01-22T15:41:47+01:00

admin commented

2026-01-22 15:41:47 +01:00

Originally created by @kiler129 on GitHub (Mar 2, 2022).

Feature Request

Q	A
New Feature	yes
RFC	yes
BC Break	yes/no

Summary

When relationships are defined the referencedColumnName is automatically derived from the target entity, unless JoinColumn is specified. When JoinColumn is added for any other purpose the referencedColumnName is hardcoded to id. I think it should be fetched from the target entity as if no JoinColumn was defined.

Reasoning

When defining relationships, e.g.:

    #[ORM\OneToOne(inversedBy: 'media')]
    #[ORM\JoinColumn(name: 'ZATTACHMENT1')]
    public Attachment $attachment;

A lot of things are assumed:

type of the relationship from PHP type
nullability
target entity (based on type)
column name (based on property name transformed by naming strategy via joinColumnName())
reference column name on the other side (based on target entity PK, optionally derived from naming strategy referenceColumnName())

Normally entities also get their primary key column name from the naming strategy, making the JoinColumn optional. However, as soon as it gets added for a different purpose (like above: to change column name) it creates a strange side effect of referencedColumnName being overridden by a hardcoded value of id:

84df37de97/lib/Doctrine/ORM/Mapping/JoinColumn.php (L43-L45)

Possible improvements

I'm sure there was a reason why this decision was made, but I cannot find it.

The ideal solution, in my opinion, will be defaulting the referencedColumnName to the target entity primary key, in the same was as when JoinColumn isn't present on the property. However, this may be seen as a BC break. Changing it now to suddenly read from the naming strategy/target entity may not be the best course of action as applications may rely on this behavior (even if a small number).

However, it's also extremely unintuitive (especially when using PHPDoc-based annotations) to see the referenced column change when the user think adding JoinColumn is added to only change the column name. The alternative could be explicitly setting the parameter to null to indicate "figure this out yourself". Currently it's impossible to achieve that - one has to manually specify referencedColumnName every time JoinColumn is used.

WDYT? Can this be improved? Currently it leads to a lot of boilerplate code and semi-hardcoded PK name outside of a custom naming strategy.

Originally created by @kiler129 on GitHub (Mar 2, 2022). ### Feature Request | Q | A |------------ | ------ | New Feature | yes | RFC | yes | BC Break | yes/no #### Summary When relationships are defined the `referencedColumnName` is automatically derived from the target entity, unless `JoinColumn` is specified. When `JoinColumn` is added for any other purpose the `referencedColumnName` is hardcoded to `id`. I think it should be fetched from the target entity as if no `JoinColumn` was defined. #### Reasoning When defining relationships, e.g.: ```php #[ORM\OneToOne(inversedBy: 'media')] #[ORM\JoinColumn(name: 'ZATTACHMENT1')] public Attachment $attachment; ``` A lot of things are assumed: - type of the relationship from PHP type - nullability - target entity (based on type) - column name (based on property name transformed by naming strategy via `joinColumnName()`) - reference column name on the other side (based on target entity PK, optionally derived from naming strategy `referenceColumnName()`) Normally entities also get their primary key column name from the naming strategy, making the `JoinColumn` optional. However, as soon as it gets added for a different purpose (like above: to change column name) it creates a strange side effect of `referencedColumnName` being overridden by a hardcoded value of `id`: https://github.com/doctrine/orm/blob/84df37de97d5cdac545f4070ec90a7279f806949/lib/Doctrine/ORM/Mapping/JoinColumn.php#L43-L45 #### Possible improvements I'm sure there was a reason why this decision was made, but I cannot find it. The ideal solution, in my opinion, will be defaulting the `referencedColumnName` to the target entity primary key, in the same was as when `JoinColumn` isn't present on the property. However, this may be seen as a BC break. Changing it now to suddenly read from the naming strategy/target entity may not be the best course of action as applications may rely on this behavior (even if a small number). However, it's also extremely unintuitive (especially when using PHPDoc-based annotations) to see the referenced column change when the user think adding `JoinColumn` is added to only change the column name. The alternative could be explicitly setting the parameter to `null` to indicate *"figure this out yourself"*. Currently it's impossible to achieve that - one has to manually specify `referencedColumnName` every time `JoinColumn` is used. WDYT? Can this be improved? Currently it leads to a lot of boilerplate code and semi-hardcoded PK name outside of a custom naming strategy.

admin commented

2026-01-22 15:41:48 +01:00

@curry684 commented on GitHub (Oct 20, 2022):

Related to this I just ran into a similarly annoying issue - the fact that 'id' is hardcoded as the referenced column name means that naming strategy is bypassed. I have a project that, for legacy reasons, has a naming strategy with leading caps:

    /**
     * Returns the default reference column name.
     *
     * @return string A column name
     */
    public function referenceColumnName(): string
    {
        return 'Id';
    }

    /**
     * Returns a join column name for a property.
     *
     * @param string $propertyName A property name
     *
     * @return string A join column name
     */
    public function joinColumnName($propertyName)
    {
        return ucfirst($propertyName) . $this->referenceColumnName();
    }

Now not adding a JoinColumn attribute works fine, however when I want to make the join not nullable:

#[JoinColumn(nullable: false)]

I get a mapping violation:

 * The referenced column name 'id' has to be a primary key column on the target entity class '...'.

The fix for this is identical to what @kiler129 mentions above - the default should not be 'id' but NULL, indicating "please figure out a sane default using the name within the configured naming strategy".

@curry684 commented on GitHub (Oct 20, 2022): Related to this I just ran into a similarly annoying issue - the fact that `'id'` is hardcoded as the referenced column name means that naming strategy is bypassed. I have a project that, for legacy reasons, has a naming strategy with leading caps: ``` /** * Returns the default reference column name. * * @return string A column name */ public function referenceColumnName(): string { return 'Id'; } /** * Returns a join column name for a property. * * @param string $propertyName A property name * * @return string A join column name */ public function joinColumnName($propertyName) { return ucfirst($propertyName) . $this->referenceColumnName(); } ``` Now *not* adding a `JoinColumn` attribute works fine, however when I want to make the join not nullable: ``` #[JoinColumn(nullable: false)] ``` I get a mapping violation: ``` * The referenced column name 'id' has to be a primary key column on the target entity class '...'. ``` The fix for this is identical to what @kiler129 mentions above - the default should not be `'id'` but `NULL`, indicating "please figure out a sane default using the name *within the configured naming strategy*".

admin commented

2026-01-22 15:41:48 +01:00

@k00ni commented on GitHub (Jul 3, 2024):

Any news on this?

@k00ni commented on GitHub (Jul 3, 2024): Any news on this?

admin commented

2026-01-22 15:41:49 +01:00

@siggidiel commented on GitHub (Dec 19, 2024):

Same here. We have a custom naming strategy which prefixes all columns of a table with the table name. So, also the ID column ends up with the name tableb_id. And if we have a relation from tablea to tableb, we are providing the name like this: tablea_tableb_id just to directly show that this column is a foreign key to TableB.

But inside the entity TableA (owning side) the annotation/attribute JoinColumn(referenceColumnName: 'tableb_id') needs to be set explicitly because the ID string is hardcoded as already mentioned by the thread creator. So, because the method public function referenceColumnName(): string from the NamingStrategy interface does not provide any method arguments to dynamically format your reference column name, it's always necessary to explicitly set the annotation in the entities.

@siggidiel commented on GitHub (Dec 19, 2024): Same here. We have a custom naming strategy which prefixes all columns of a table with the table name. So, also the ID column ends up with the name `tableb_id`. And if we have a relation from `tablea` to `tableb`, we are providing the name like this: `tablea_tableb_id` just to directly show that this column is a foreign key to TableB. But inside the entity TableA (owning side) the annotation/attribute `JoinColumn(referenceColumnName: 'tableb_id')` needs to be set explicitly because the ID string is hardcoded as already mentioned by the thread creator. So, because the method `public function referenceColumnName(): string` from the NamingStrategy interface does not provide any method arguments to dynamically format your reference column name, it's always necessary to explicitly set the annotation in the entities.

admin commented

2026-01-22 15:41:49 +01:00

@curry684 commented on GitHub (Dec 19, 2024):

As the current behaviour is downright broken I do think something should be done about it, however technically the change is breaking, even though I do not think there's any code out there relying on the id default and having a custom naming strategy, so it will not actually break any real world code.

The fix is fairly easy but, knowing Doctrine's stance on BC, it will likely have to await a new major.

@curry684 commented on GitHub (Dec 19, 2024): As the current behaviour is downright broken I do think something should be done about it, however technically the change is breaking, even though I do not think there's any code out there relying on the `id` default *and* having a custom naming strategy, so it will not *actually* break any real world code. The fix is fairly easy but, knowing Doctrine's stance on BC, it will likely have to await a new major.

admin commented

2026-01-22 15:41:49 +01:00

@greg0ire commented on GitHub (Dec 19, 2024):

Possible next steps:

provide a PHPUnit test allowing to reproduce the bug
provide a naive, BC-breaking fix demonstrating what we should end up with in 4.0.0

When that's done, we can try to imagine ways to provide the same fix without a breaking change, like maybe introducing a new attribute just for this, or a temporary extra argument that you could invoke like so: #[ORM\JoinColumn(name: 'ZATTACHMENT1', detect_pk: true)]

@greg0ire commented on GitHub (Dec 19, 2024): Possible next steps: - provide a PHPUnit test allowing to reproduce the bug - provide a naive, BC-breaking fix demonstrating what we should end up with in 4.0.0 When that's done, we can try to imagine ways to provide the same fix without a breaking change, like maybe introducing a new attribute just for this, or a temporary extra argument that you could invoke like so: `#[ORM\JoinColumn(name: 'ZATTACHMENT1', detect_pk: true)]`

admin commented

2026-01-22 15:41:50 +01:00

@curry684 commented on GitHub (Dec 20, 2024):

Adding it to every JoinColumn would hardly be better than just adding the referencedColumnName :)

I would prefer a config level switch to fix the behavior in a minor and flip the default in ORM 4, since having a naming strategy is also done there:

doctrine:
    orm:
        naming_strategy: My\Project\Doctrine\ORM\MyNamingStrategy
        references_follow_naming_strategy: true

@curry684 commented on GitHub (Dec 20, 2024): Adding it to every `JoinColumn` would hardly be better than just adding the `referencedColumnName` :) I would prefer a config level switch to fix the behavior in a minor and flip the default in ORM 4, since having a naming strategy is also done there: ```yaml doctrine: orm: naming_strategy: My\Project\Doctrine\ORM\MyNamingStrategy references_follow_naming_strategy: true ```

admin commented

2026-01-22 15:41:50 +01:00

@greg0ire commented on GitHub (Dec 20, 2024):

Yeah that would be great indeed! To that end, it would be great to locate the piece of code where the autodetection happens.

@greg0ire commented on GitHub (Dec 20, 2024): Yeah that would be great indeed! To that end, it would be great to locate the piece of code where the autodetection happens.

admin commented

2026-01-22 15:41:50 +01:00

@curry684 commented on GitHub (Dec 20, 2024):

I think the fix itself is easier than anticipated: 722969909b

Is there a timeline on ORM4? Ie. - is it even worth implementing a B/C layer (which would be far more work and involve DoctrineBundle as well)

@curry684 commented on GitHub (Dec 20, 2024): I think the fix itself is easier than anticipated: https://github.com/curry684/orm/commit/722969909b7d17e8450683e38b94978a36794ddd Is there a timeline on ORM4? Ie. - is it even worth implementing a B/C layer (which would be far more work and involve DoctrineBundle as well)

admin commented

2026-01-22 15:41:51 +01:00

@greg0ire commented on GitHub (Dec 20, 2024):

No clear timeline yet, right now we're trying to get ORM 3 adopted, it is still young. I can't imagine either how it would break existing code, so I'd say let's target 3.4.x, and be prepared to revert this upon release if we see complaints / lectures about semver and whether we know what it means.

@greg0ire commented on GitHub (Dec 20, 2024): No clear timeline yet, right now we're trying to get ORM 3 adopted, it is still young. I can't imagine either how it would break existing code, so I'd say let's target 3.4.x, and be prepared to revert this upon release if we see complaints / lectures about semver and whether we know what it means.

admin commented

2026-01-22 15:41:51 +01:00

@curry684 commented on GitHub (Dec 20, 2024):

Ok, in that case I'll see if I can whip up some tests for the change :)

(most important test first: I'll symlink this build into the one project I have with a custom strategy)

@curry684 commented on GitHub (Dec 20, 2024): Ok, in that case I'll see if I can whip up some tests for the change :) (most important test first: I'll symlink this build into the one project I have with a custom strategy)

admin commented

2026-01-22 15:41:52 +01:00

@curry684 commented on GitHub (Dec 21, 2024):

Tests added, seems to work fine for all drivers.

(most important test first: I'll symlink this build into the one project I have with a custom strategy)

This test also succeeded - copying my PR build of ORM into the project and removing the referencedColumnName declarations from all JoinColumn attributes results in an empty changeset to the database without errors.

PR targets 3.4 as suggested above.

@curry684 commented on GitHub (Dec 21, 2024): Tests added, seems to work fine for all drivers. > (most important test first: I'll symlink this build into the one project I have with a custom strategy) This test also succeeded - copying my PR build of ORM into the project and removing the `referencedColumnName` declarations from all `JoinColumn` attributes results in an empty changeset to the database without errors. PR targets 3.4 as suggested above.

admin commented

2026-01-22 15:41:52 +01:00

@curry684 commented on GitHub (Dec 23, 2024):

For completion's sake: this issue is only partially resolved by my PR as @ServerExe in https://github.com/doctrine/orm/issues/9558#issuecomment-2553386677 requests functionality to determine the referencedColumnName based on the relation. As this requires changing the NamingStrategy interface it's highly breaking and can only be done in a major.

The change is made a lot easier by my PR though if someone wants to pick up from here.

@curry684 commented on GitHub (Dec 23, 2024): For completion's sake: this issue is *only partially resolved by my PR* as @ServerExe in https://github.com/doctrine/orm/issues/9558#issuecomment-2553386677 requests functionality to determine the `referencedColumnName` based on the relation. As this requires changing the `NamingStrategy` interface it's highly breaking and can only be done in a major. The change is made a lot easier by my PR though if someone wants to pick up from here.

admin commented

2026-01-22 15:41:52 +01:00

@siggidiel commented on GitHub (Dec 23, 2024):

For completion's sake: this issue is only partially resolved by my PR as @ServerExe in https://github.com/doctrine/orm/issues/9558#issuecomment-2553386677 requests functionality to determine the referencedColumnName based on the relation. As this requires changing the NamingStrategy interface it's highly breaking and can only be done in a major.

The change is made a lot easier by my PR though if someone wants to pick up from here.

Hey @curry684,

thanks for your response. It isn't necessarily a request of mine. I would be fine with other solutions. As long as the referenced column name can be determined somehow, I would appreciate it :) But I am also fine with a change in the next Major update. In the meantime, we can live with explicitly providing the name in the PHP Attribute.

@siggidiel commented on GitHub (Dec 23, 2024): > For completion's sake: this issue is *only partially resolved by my PR* as @ServerExe in https://github.com/doctrine/orm/issues/9558#issuecomment-2553386677 requests functionality to determine the `referencedColumnName` based on the relation. As this requires changing the `NamingStrategy` interface it's highly breaking and can only be done in a major. > > The change is made a lot easier by my PR though if someone wants to pick up from here. Hey @curry684, thanks for your response. It isn't necessarily a request of mine. I would be fine with other solutions. As long as the referenced column name can be determined somehow, I would appreciate it :) But I am also fine with a change in the next Major update. In the meantime, we can live with explicitly providing the name in the PHP Attribute.

admin commented

2026-01-22 15:41:53 +01:00

@curry684 commented on GitHub (Dec 23, 2024):

To be honest I think, given how exotic custom naming strategies are, likely only being used for legacy databases, I don't think this request is going to get much love from the team for the next major.

Technically, we could add an interface parameter in PHP without B/C issues due to how PHP handles extra parameters in function calls, so you could attempt an implementation.

@curry684 commented on GitHub (Dec 23, 2024): To be honest I think, given how exotic custom naming strategies are, likely only being used for legacy databases, I don't think this request is going to get much love from the team for the next major. Technically, we could add an interface parameter in PHP without B/C issues due to how PHP handles extra parameters in function calls, so you could attempt an implementation.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6940

Set JoinColumn::referencedColumnName based on the target entity instead of id #6940

Feature Request

Summary

Reasoning

Possible improvements

Set `JoinColumn::referencedColumnName` based on the target entity instead of `id` #6940