mirror of
https://github.com/doctrine/orm.git
synced 2026-03-24 06:52:09 +01:00
[RFC] Read nullability from PHP type #6975
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @rixafy on GitHub (May 9, 2022).
Feature Request
I would like to have nullable columns according to my PHP nullable properties, when column property is nullable, column should be also nullable, if there is no nullable attribute that says otherwise, I checked my entities in multiple big projects, and I have 1:1 nullable database values with nullable PHP properties.
This code should result in column being nullable in database, if there was declared
#[Column(nullable: false)], column would not be nullable, but PHP variable can be, what is really bad practice, since entity can get into invalid state, and if it will be saved, application runs into an error. I thought about it, and I can't come up with one example where it's good to have nullable field in PHP, but not in database.Previous discussions
This matter was discussed multiple times, once, this feature it was shipped into the
2.9.1version, so some programmers (https://github.com/doctrine/orm/issues/8770, https://github.com/doctrine/orm/issues/8834) (me including) have removednullableproperty from Column annotation, in version2.9.2it was reverted (https://github.com/doctrine/orm/pull/8732) and a BC break was introduced. I doubt there was many people that had different nullability in PHP.Currently it's stated that it is not in
2.xbecause of backward compatibility (https://github.com/doctrine/orm/pull/8835), and main issue that caused rollback of the feature was this https://github.com/doctrine/orm/issues/8723 (and I think that only that one issue). And in that issue it was stated by @beberlei https://github.com/doctrine/orm/issues/8723#issuecomment-851282446 that you can look at it in ORM 3, what would be great, but then I asked about it recently, and got answer https://github.com/doctrine/orm/issues/9736#issuecomment-1120356110So, I'm curious based on what did you decide to not inherit nullability from PHP types?
Motivation
My vision is to have clean code, less bugs and less repeating declarations, ORM has been been going this way last couple of years.
For instance:
#[Column]attribute.#[Column], not even enumType, since it's inherited from PHP type#[Embedded]targetEntityin ManyToOne relationships, it's inherited from PHP type.Also it enforces entity valid state, because if PHP value could be null and if there was non-null column in database, after flushing the entity there will be a SQL error. So there is that.
And if someone want nullable type to be non-nullable in database, they can simply declare
nullable: falsein#[Column]attribute.Another thing is, that happened to me a few times, that I totally forgot to declare nullable columns in annotations additionally, and I discovered it after I wanted to save entity, that PHP values could be nullable, but database columns could not.
Here is an example of that mentioned code where I needed to add additional

nullableattribute and how bad it looksAnd here is same code without that attribute nullability declarations

@beberlei commented on GitHub (May 9, 2022):
The problems we face are:
ManyToOneandOneToOnewhich are nullable by default, however properties in PHP are not nullable by default. Should we change the behavior of both assocations to be non nullable by default?It would be technically feasible to use null or not null as the default for
nullableoption and only override it when using the option explicitly. The problem why this was reverted ultimateily is, how do we get there from the ORM 2 behavior without f***ing everyone's existing code up.@rixafy commented on GitHub (May 9, 2022):
Well, when someone updates from 2.0 to 3.0, BC breaks are expected, I bet that most people will have not a single line in migration, since now they are forced to declare nullability via attribute, it would mean that they have nullable property and non-nullable column in database, what would result in error when saving entity with null value, so they will have maybe few lines in migration and they check the entity properties, figure out only they are nullable, and therefore they add
nullable: falseor they realize this shouldn't be nullable in PHP or should be nullable in database.In conclusion, that migration will probably help them discover bugs.
Another deprecating solution is to add deprecated yellow warnings when generating migrations or orm schema update, that there are entity properties that are nullable but database nullability doesn't match, so they should add
nullable: falseto entity attribute/annotation, because in 3.0, sql commands will be generated.I think that
ManyToOneandOneToOneshould stay nullable by default for a while, that's something else and those are not regular column types but relations, that would be way too big BC break and I don't think users are ready for it, maybe it could be discussed and introduced in next version such as 4.0.But on the other hand, if you make
ManyToOneandOneToOnerespect nullability, much more SQL would be generated in migration, therefore, there is not a chance that someone will overlook 1 SQL command, because there will be plenty of them and programmers will realize something has changed, so they visit ORM 3.0 docs where will beUpgrading from 2.xsection and they figure out that php nullability is preferred.I personally wouldn't have problem with relations being non-nullable, it would bring more consistency and less bugs, since now if I set somehow column in db to be null, ORM will probably scream that there is non-nullable variable and null was passed to that variable.
Another plus is that there can't be an empty value of
""in database relation, because it would result in foreign constraint fail so only valid value would be some id from another table.@derrabus commented on GitHub (May 9, 2022):
Yes, but they have to be prepared. This means that there has to be some way to opt-in to the new behavior before it becomes mandatory with the major release. There are still applications out there using Zend Framework 1, Symfony 1 and/or Doctrine 1 because the upgrade path of each of those libraries to version 2 had been too difficult. The PHP ecosystem has learnt from its mistakes.
You will lose that bet. A newly created entity does not necessarily need to be ready to be persited right away. I have seen quite a few codebases in the past where Entities were created like this:
In that case,
nullis a valid property state for the entity class itself (because it may represent an incomplete record) although the corresponding database column does not accept null. And yes, such an incomplete entity cannot be persited, but that would be intentional in that case.This is probably not an approach you would take and I respect that. But that does not render this approach invalid.
A special case of that are generated primary keys like this one:
Here, the database column must not be nullable and an entity with
$id === nullcan actually be persited.So to conclude: even if we decide to guess nullability from the property type in 3.0, a proper deprecation layer has to be in place that notifies the developer that his current mapping configuration will trigger different behavior in 3.0. And here, the change is especially tricky because you want to change the semantics of the omission of the
nullableproperty from well-defined defaults to a detection logic.That might be a nice bonus, but using Doctrine Migrations is completely optional which is why our deprecation layer must not depend on it.
Taking into account the type of codebase I mentioned earlier, adding this kind of nullability guessing to relations is not particularly heavier than for regular columns.
@rixafy commented on GitHub (May 9, 2022):
That special cases with
$idcan be resolved with some exception forId? ThatIdjust can't be nullable in any scenario?That's sounds good, if there was option in configuration, for example
preferPhpNullability, it could be added into2.xwithfalseas default value, and in ORM 3, the default value would betrue, so if someone has trouble with upgrade, they can switch back tofalse. What would be mentioned in the docs in theUpgrade from 2.xsection.@rixafy commented on GitHub (Jan 28, 2025):
Hi @derrabus, do you think it's reasonable to add this at least as opt-in in configuration? It would help a lot to have cleaner entity properties, that's one of the few things why do I have something in JoinColumn or Column attribute.
Regarding that
#[Id]attribute, I think we should make no exception, and inherit it as well (when configured),$idshould not have nullable data type, because it should not be accessed before initialization. And if some reason it does have nullable PHP type, you can always change it with#[Column(nullable: false)].@derrabus commented on GitHub (Jan 28, 2025):
We can do that, yes. But someone would need to work on the feature and apparently nobody did that during the last three years.
@rixafy commented on GitHub (Jan 28, 2025):
It has already been done, but it was reverted https://github.com/doctrine/orm/pull/8732, I tried to argue that we could have it at least opt in, so if it's possible, I'll just write same implementation when I'll find some free time, I'll just add configuration support and default option will be current behavior.