Upgrade from 2.8.x to 2.9.1: possible BC break for nullable behaviour #6736

New Issue

admin · 2026-01-22T15:37:42+01:00

admin commented

2026-01-22 15:37:42 +01:00

Originally created by @jmsche on GitHub (May 25, 2021).

Hi everyone,

I just upgraded doctrine/orm to 2.9.1 (from 2.8.x) on a simple project.

It makes some columns that were previously not nullable, nullable (the Column annotation does not have "nullable" set to any value in 2.9, but from what I can see it was false by default until 2.8.x included).

I think the changes happened since these PRs: https://github.com/doctrine/orm/pull/8439 & https://github.com/doctrine/orm/pull/8589

I guess the changes happen because the property in my code is nullable, and as nullable is not defined in my Column annotation, the driver thinks it's nullable.

Is it considered a BC break?

Regards,

Originally created by @jmsche on GitHub (May 25, 2021). Hi everyone, I just upgraded doctrine/orm to 2.9.1 (from 2.8.x) on a simple project. It makes some columns that were previously not nullable, nullable (the Column annotation [does not have "nullable" set to any value in 2.9](https://github.com/doctrine/orm/blob/2.9.x/lib/Doctrine/ORM/Mapping/Column.php#L61), but from what I can see [it was false by default until 2.8.x included](https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/Mapping/Column.php#L56)). I think the changes happened since these PRs: https://github.com/doctrine/orm/pull/8439 & https://github.com/doctrine/orm/pull/8589 I guess the changes happen because the property in my code is nullable, and as nullable is not defined in my Column annotation, the driver thinks it's nullable. Is it considered a BC break? Regards,

admin closed this issue

2026-01-22 15:37:42 +01:00

admin commented

2026-01-22 15:37:43 +01:00

@zolex commented on GitHub (May 25, 2021):

I can confirm this behavior. also the JoinColumn(nullable=true) seems to be ignored and also the migrations creates update table statements that set nullable to false where it should be true.

@zolex commented on GitHub (May 25, 2021): I can confirm this behavior. also the `JoinColumn(nullable=true)` seems to be ignored and also the migrations creates update table statements that set nullable to false where it should be true.

admin commented

2026-01-22 15:37:43 +01:00

@beberlei commented on GitHub (May 25, 2021):

I thought I adressed this with https://github.com/doctrine/orm/pull/8678 - can you check how this relates?

@beberlei commented on GitHub (May 25, 2021): I thought I adressed this with https://github.com/doctrine/orm/pull/8678 - can you check how this relates?

admin commented

2026-01-22 15:37:43 +01:00

@jmsche commented on GitHub (May 25, 2021):

I think there are two different bugs here (but maybe related):

Initial issue about @Column setting nullable to true
@zolex's issue about @JoinColumn which can't override nullable

@jmsche commented on GitHub (May 25, 2021): I think there are two different bugs here (but maybe related): - Initial issue about `@Column` setting nullable to true - @zolex's issue about `@JoinColumn` which can't override nullable

admin commented

2026-01-22 15:37:44 +01:00

@dmaicher commented on GitHub (May 25, 2021):

Similar problem here with 2.9.1 (upgraded from 2.8.5):

I have an entity with a relation like

/**
 * @ORM\Entity
 */
class InheritedQuestionnaire extends AbstractQuestionnaire
{
    /**
     * @ORM\JoinColumn(nullable=true)
     * @ORM\OneToOne(targetEntity="Questionnaire")
     */
    private Questionnaire $parentQuestionnaire;

    public function __construct(Questionnaire $parentQuestionnaire)
    {
        $this->parentQuestionnaire = $parentQuestionnaire;
    }
 
    // ...   
}

But since its using single table inheritance the column needs to be nullable. Only for this particular type its set. Now the nullable=true is ignored and I get a migration diff like

$this->addSql('ALTER TABLE questionnaire CHANGE parent_questionnaire_id parent_questionnaire_id INT NOT NULL');

@dmaicher commented on GitHub (May 25, 2021): Similar problem here with 2.9.1 (upgraded from 2.8.5): I have an entity with a relation like ```php /** * @ORM\Entity */ class InheritedQuestionnaire extends AbstractQuestionnaire { /** * @ORM\JoinColumn(nullable=true) * @ORM\OneToOne(targetEntity="Questionnaire") */ private Questionnaire $parentQuestionnaire; public function __construct(Questionnaire $parentQuestionnaire) { $this->parentQuestionnaire = $parentQuestionnaire; } // ... } ``` But since its using single table inheritance the column needs to be nullable. Only for this particular type its set. Now the `nullable=true` is ignored and I get a migration diff like ```php $this->addSql('ALTER TABLE questionnaire CHANGE parent_questionnaire_id parent_questionnaire_id INT NOT NULL'); ```

admin commented

2026-01-22 15:37:44 +01:00

@pieterw2w commented on GitHub (May 26, 2021):

same issue as dmaicher: the column is no longer nullable.

@pieterw2w commented on GitHub (May 26, 2021): same issue as dmaicher: the column is no longer nullable.

admin commented

2026-01-22 15:37:45 +01:00

@TheKassaK commented on GitHub (May 26, 2021):

Same issue, downgraded to 2.8.5

@TheKassaK commented on GitHub (May 26, 2021): Same issue, downgraded to 2.8.5

admin commented

2026-01-22 15:37:45 +01:00

@Lustmored commented on GitHub (May 27, 2021):

@beberlei I'm afraid #8678 introduced at least one part of this problem. Change in lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php made it overwrite nullable attribute whenever non-nullable typed parameter is found.

This is because when nullable on JoinColumn annotation is non-nullable bool there is no way telling if it is being set by user or have default value and the overwrite happens every time.

Solving it the right way would probably require nullable on JoinColumn to be nullable again (and fighting strange bugs I didn't quite understood in other ORM parts) or using some other way to determine whether it was set or not or removing this rule and reintroduce it in 3.0 in some way that wouldn't enforce BC.

I can try to pursue "some other way to determine whether it was set or not" option but I am a bit concerned about mapping drivers that don't use JoinColumn class (YAML/XML/PHP) - I have to investigate them before judging.

@Lustmored commented on GitHub (May 27, 2021): @beberlei I'm afraid #8678 introduced at least one part of this problem. Change in `lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php` made it overwrite `nullable` attribute whenever non-nullable typed parameter is found. This is because when `nullable` on `JoinColumn` annotation is non-nullable bool there is no way telling if it is being set by user or have default value and the overwrite happens every time. Solving it the right way would probably require `nullable` on `JoinColumn` to be nullable again (and fighting strange bugs I didn't quite understood in other ORM parts) or using some other way to determine whether it was set or not or removing this rule and reintroduce it in 3.0 in some way that wouldn't enforce BC. I can try to pursue "some other way to determine whether it was set or not" option but I am a bit concerned about mapping drivers that don't use `JoinColumn` class (YAML/XML/PHP) - I have to investigate them before judging.

admin commented

2026-01-22 15:37:45 +01:00

@Lustmored commented on GitHub (May 27, 2021):

I have created draft of the possible solution that minimizes BC impact and fixes overwriting nullable parameter. I would love getting some feedback before working on remaining drivers.

This however does not change default @Column behavior as it seems to defy it's purpose. Non-nullable typed property should in my opinion be not nullable in database by default, because passing null to non-nullable property throws TypeError as far as I know.

@Lustmored commented on GitHub (May 27, 2021): I have created draft of the possible solution that minimizes BC impact and fixes overwriting `nullable` parameter. I would love getting some feedback before working on remaining drivers. This however does not change default `@Column` behavior as it seems to defy it's purpose. Non-nullable typed property should in my opinion be not nullable in database by default, because passing null to non-nullable property throws `TypeError` as far as I know.

admin commented

2026-01-22 15:37:45 +01:00

@dmaicher commented on GitHub (May 27, 2021):

Non-nullable typed property should in my opinion be not nullable in database by default, because passing null to non-nullable property throws TypeError as far as I know.

@Lustmored but this is tricky when talking about single table entity inheritance as in my case mentioned above. There the column needs to be nullable for other types of the entity although its property is typed and not nullable

@dmaicher commented on GitHub (May 27, 2021): > Non-nullable typed property should in my opinion be not nullable in database by default, because passing null to non-nullable property throws TypeError as far as I know. @Lustmored but this is tricky when talking about single table entity inheritance as in my case mentioned above. There the column needs to be nullable for other types of the entity although its property is typed and not nullable

admin commented

2026-01-22 15:37:45 +01:00

@Lustmored commented on GitHub (May 27, 2021):

Non-nullable typed property should in my opinion be not nullable in database by default, because passing null to non-nullable property throws TypeError as far as I know.

@Lustmored but this is tricky when talking about single table entity inheritance as in my case mentioned above. There the column needs to be nullable for other types of the entity although its property is typed and not nullable

This is covered by my draft change where in your example column will still be nullable. The change is about defaults and it failed with JoinColumn, where defaults were enforced by mistake.

So your example:

/**
 * @ORM\Entity
 */
class InheritedQuestionnaire extends AbstractQuestionnaire
{
    /**
     * @ORM\JoinColumn(nullable=true)
     * @ORM\OneToOne(targetEntity="Questionnaire")
     */
    private Questionnaire $parentQuestionnaire;

    public function __construct(Questionnaire $parentQuestionnaire)
    {
        $this->parentQuestionnaire = $parentQuestionnaire;
    }
 
    // ...   
}

will work with my draft, while:

/**
 * @ORM\Entity
 */
class InheritedQuestionnaire extends AbstractQuestionnaire
{
    /**
     * @ORM\JoinColumn
     * @ORM\OneToOne(targetEntity="Questionnaire")
     */
    private Questionnaire $parentQuestionnaire;

    public function __construct(Questionnaire $parentQuestionnaire)
    {
        $this->parentQuestionnaire = $parentQuestionnaire;
    }
 
    // ...   
}

will still default to nullable: false).

Hope that makes it clear :) I'm no expert here, so possibly this approach have some drawbacks too.

@Lustmored commented on GitHub (May 27, 2021): > > Non-nullable typed property should in my opinion be not nullable in database by default, because passing null to non-nullable property throws TypeError as far as I know. > > @Lustmored but this is tricky when talking about single table entity inheritance as in my case mentioned above. There the column needs to be nullable for other types of the entity although its property is typed and not nullable This is covered by my draft change where in your example column will still be nullable. The change is about defaults and it failed with `JoinColumn`, where defaults were enforced by mistake. So your example: ``` /** * @ORM\Entity */ class InheritedQuestionnaire extends AbstractQuestionnaire { /** * @ORM\JoinColumn(nullable=true) * @ORM\OneToOne(targetEntity="Questionnaire") */ private Questionnaire $parentQuestionnaire; public function __construct(Questionnaire $parentQuestionnaire) { $this->parentQuestionnaire = $parentQuestionnaire; } // ... } ``` will work with my draft, while: ``` /** * @ORM\Entity */ class InheritedQuestionnaire extends AbstractQuestionnaire { /** * @ORM\JoinColumn * @ORM\OneToOne(targetEntity="Questionnaire") */ private Questionnaire $parentQuestionnaire; public function __construct(Questionnaire $parentQuestionnaire) { $this->parentQuestionnaire = $parentQuestionnaire; } // ... } ``` will still default to `nullable: false`). Hope that makes it clear :) I'm no expert here, so possibly this approach have some drawbacks too.

admin commented

2026-01-22 15:37:46 +01:00

@beberlei commented on GitHub (May 29, 2021):

I will look into this and @Lustmored draft PR the coming days, this has highest priority to fix with a release planned for next week.

@beberlei commented on GitHub (May 29, 2021): I will look into this and @Lustmored draft PR the coming days, this has highest priority to fix with a release planned for next week.

admin commented

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

@Lustmored commented on GitHub (May 29, 2021):

I will look into this and @Lustmored draft PR the coming days, this has highest priority to fix with a release planned for next week.

I can manage time to work on this PR on Monday, so if idea behind it seems right let me know and I'll finish it.

@Lustmored commented on GitHub (May 29, 2021): > I will look into this and @Lustmored draft PR the coming days, this has highest priority to fix with a release planned for next week. I can manage time to work on this PR on Monday, so if idea behind it seems right let me know and I'll finish it.

admin commented

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

@beberlei commented on GitHub (May 31, 2021):

I think we need to remove the nullable/ˋallowsNullˋ code completely, because as said you might want toconfigure this differently in DB and code

@beberlei commented on GitHub (May 31, 2021): I think we need to remove the nullable/ˋallowsNullˋ code completely, because as said you might want toconfigure this differently in DB and code

admin commented

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

@Lustmored commented on GitHub (May 31, 2021):

@beberlei How do you feel about leaving this logic only for Attribute driver? It is new in 2.9 (no BC break), requires PHP8 (so typed properties will be common there) and could simply be useful to new users. I could get working PR today.

Otherwise - is this change something you'd like to see for ORM 3?

@Lustmored commented on GitHub (May 31, 2021): @beberlei How do you feel about leaving this logic only for Attribute driver? It is new in 2.9 (no BC break), requires PHP8 (so typed properties will be common there) and could simply be useful to new users. I could get working PR today. Otherwise - is this change something you'd like to see for ORM 3?

admin commented

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

@zolex commented on GitHub (May 31, 2021):

@beberlei How do you feel about leaving this logic only for Attribute driver? It is new in 2.9 (no BC break), requires PHP8 (so typed properties will be common there) and could simply be useful to new users. I could get working PR today.

Otherwise - is this change something you'd like to see for ORM 3?

I don't think it's good idea to have different logic for the same definitions in annotations and attributes.

@zolex commented on GitHub (May 31, 2021): > @beberlei How do you feel about leaving this logic only for Attribute driver? It is new in 2.9 (no BC break), requires PHP8 (so typed properties will be common there) and could simply be useful to new users. I could get working PR today. > > Otherwise - is this change something you'd like to see for ORM 3? I don't think it's good idea to have different logic for the same definitions in annotations and attributes.

admin commented

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

@beberlei commented on GitHub (May 31, 2021):

I agree with @zolex the logic should be consistent across drivers. We could look into this for ORM 3 though. At least if nullable is not set and the field allows null, then nullable should default to true. A user could always change it back to be nullable=false.

@beberlei commented on GitHub (May 31, 2021): I agree with @zolex the logic should be consistent across drivers. We could look into this for ORM 3 though. At least if nullable is not set and the field allows null, then nullable should default to true. A user could always change it back to be nullable=false.

admin commented

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

@Lustmored commented on GitHub (May 31, 2021):

I agree with @zolex the logic should be consistent across drivers. We could look into this for ORM 3 though. At least if nullable is not set and the field allows null, then nullable should default to true. A user could always change it back to be nullable=false.

All right, thanks for the answer. I will work towards PR targeting 3.0 👍

@Lustmored commented on GitHub (May 31, 2021): > I agree with @zolex the logic should be consistent across drivers. We could look into this for ORM 3 though. At least if nullable is not set and the field allows null, then nullable should default to true. A user could always change it back to be nullable=false. All right, thanks for the answer. I will work towards PR targeting 3.0 :+1:

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#6736