[Django] #30610: Model.full_clean() can ignore invalid (null) values which should raise ValidationError, because of blank=True short-circuiting

Django

unread,

Jul 3, 2019, 7:21:08 AM7/3/19

to django-...@googlegroups.com

#30610: Model.full_clean() can ignore invalid (null) values which should raise
ValidationError, because of blank=True short-circuiting
-------------------------------------+-------------------------------------
Reporter: Keryn | Owner: nobody
Knight |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Consider the following model definition (ignoring the merits of it):
{{{
class Example(models.Model):
txt = models.TextField(blank=True, null=False)
}}}
I'd expect to be able to do:
{{{
x = Example(txt=None)
x.full_clean()
}}}
which should raise a `ValidationError` because None is **explicitly** not
a valid value for the field (`null=False`) - but it doesn't, so
subsequently doing `x.save()` will raise `IntegrityError` when the
database (sqlite3, postgresql, I'd assume the same for mysql/mariadb)
attempts to do the insert, with something like:
{{{IntegrityError: NOT NULL constraint failed: testing_example.txt}}}
(sqlite)
{{{IntegrityError: null value in column "txt" violates not-null
constraint}}} (postgresql)

I think it's down to
* if `blank=True` then `Model.clean_fields()` short circuits because
`EMPTY_VALUES` includes `None`
* if `blank=False` it doesn't short-circuit and instead falls into
`Field.validate()` where the null check happens **before** the blank
check, so the ValidationError is correctly raised that `This field cannot
be null`

Tested against master @ 54dcfbc36780054d39299bfd1078938e5dd4e839,
encountered in staging environment in 1.11, but it looks like it goes back
to at least 2010 (commit 2f9853b2dc90f30317e0374396f08e3d142844d2)

There is this comment before bailing out early:
{{{
# Skip validation for empty fields with blank=True. The developer
# is responsible for making sure they have a valid value.
}}}
This is specious reasoning. Blank is not so super-special that it should
deserve throwing actual validation out of the window and into the mental
gymnastics necessary to decide whether or not you can actually try and
save something that ostensibly ''passed'' validation, according to both
the code and the intent therein.

--
Ticket URL: <https://code.djangoproject.com/ticket/30610>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Jul 3, 2019, 7:45:13 AM7/3/19

to django-...@googlegroups.com

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* status: new => closed
* resolution: => duplicate

Comment:

Duplicate of #22224.

--
Ticket URL: <https://code.djangoproject.com/ticket/30610#comment:1>

Django

unread,

Jul 3, 2019, 9:10:26 AM7/3/19

to django-...@googlegroups.com

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Keryn Knight):

Ah, didn't find that when searching Trac, sorry.

Perhaps it's worth pointing out that commenting out (just to see what
happens) `if f.blank and raw_value in f.empty_values: continue` only seems
to cause one test to fail (`test_validation_with_empty_blank_field`) and
whilst it may be related to this, it doesn't look it at a glance.

--
Ticket URL: <https://code.djangoproject.com/ticket/30610#comment:2>

Reply all

Reply to author

Forward