Remove null assingment check for non-nullable fields?

[Tim Graham]

There's a proposal to remove this behavior:

>>> obj.fk = None
ValueError('Cannot assign None: "Obj.fk" does not allow null values.)

(i.e. no more exception if you assign None to a non-nullable foreign key)

Rationale presented on the ticket [1]:

• Conceptually it is a programmer's responsibility to validate the data assigned to foreign key in a right moment - not necessarily during assignment to the model field.
• This behavior is not symmetrical to other database-related constraints as for example unique=True which is deferred to save() method (raising IntegrityError) or full_clean() method (raising ValidationError).
• In #13776 the decision was made that a None value for foreign key field with null=False and corresponding form field with required=False should be valid. It means that after model instantiation by such a form, the value of a given field is empty or just unset (not defined). Whatever the state is, it doesn't matter. It shows only that currently discussed check does not prevent in 100% against having foreign key field with null=False set to None. It undermines the legitimacy of presence for the discussed check.

Any concerns with this?

Its removal helps fix an edge case that regressed in Django 1.8 where if you have a model field with blank=False but required=False on the form field, an empty form value will be silently ignored and won't overwrite a model instance's value. I think it's a sufficient edge case, however, that it's not worth backporting this change (if accepted).

[1] https://code.djangoproject.com/ticket/25349

[Dheerendra Rathor]

With respect to rationale 2:

It makes sense to remove this check as database handle these checks very well. And just like uniqueness checks, it is nice to let database do its work.

With respect to rationale 1:

Since this behaviour will ask developers to change exception catching from ValueError to IntegrityError, I guess this will be a huge pain for developers to migrate considering how lazy we developers are.

In my opinion it is good to remove this check in future version (probably Django 2.0) and for now a RemovedInDjango1xxWarning can be showed with ValueError check. 

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/a79c63a4-abdf-4cbd-bf96-a4bc2d5b537b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Anssi Kääriäinen]

On Wednesday, February 3, 2016 at 1:26:26 AM UTC+2, Tim Graham wrote:

There's a proposal to remove this behavior:

>>> obj.fk = None
ValueError('Cannot assign None: "Obj.fk" does not allow null values.)

I don't see this check as necessary - if you try to save such an object you will get an error. If you don't, then there is no harm done.

 - Anssi

[Florian Apolloner]

On Wednesday, February 3, 2016 at 12:26:26 AM UTC+1, Tim Graham wrote:

There's a proposal to remove this behavior:

>>> obj.fk = None
ValueError('Cannot assign None: "Obj.fk" does not allow null values.)

Yes please, if you do IntegerField(null=False) you can still do instance.xxx = None -- only ForeignKeys are weird and check during assignment…
That said I thought we already removed that in 1.9 or 1.10?

[Tim Graham]

You're probably think of the change in 1.8.4 that moved the unsaved model instance assignment data loss check (added in Django 1.8) to Model.save() to allow easier usage of in-memory models (#25160).

The checks in question have been in Django since 1.0:
https://github.com/django/django/commit/1452d46240609ff39dacf9ea2f759ed600c2f09f