#35223 closed Cleanup/optimization (fixed)
Fields with db_default raise ValidationErrors when full_clean() called
Reported by: | Brian Ibbotson | Owned by: | bcail |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.0 |
Severity: | Release blocker | Keywords: | |
Cc: | Brian Ibbotson, bcail | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Starting to migrate models in my (large) project to use Django 5’s new db_default
attribute for fields (using Django 5.0.2), encountering behavior contrary to my expectations.
A field with db_default
raises a ValidationError
when full_clean()
called, if that field has been omitted from the create()
call.
This is (to me) unexpected behavior. Would expect that no error would be raised, the instance would be saved successfully, with the specified db_default
value set at the database level.
Has been explained to me in the Django forums that the ValidationError
is correct, that I should instead either
(1) explicitly choose to exclude
the missing fields from full_clean()
call,
(2) write a custom clean method for each field, or
(3) simply save the instance rather than calling full_clean()
Had always been impressed on me that it is best practice to always call full_clean()
on instance creation and/or update.
In either case, having to determine the missing fields and annotate the full_clean()
call or write a custom clean method per field seem to work against the usual conception of a default value, which should propagate... well, by default and allow for simpler operation where possible.
I would suggest having fields with db_default
be excluded by default from full_clean()
If the current behavior is re-affirmed, would suggest incorporating the suggested 3 workaround steps into the Django documentation, as I suspect most users would have similar expectations as myself when implementing this in future code.
Change History (13)
comment:1 by , 9 months ago
Description: | modified (diff) |
---|
comment:2 by , 9 months ago
Description: | modified (diff) |
---|
comment:3 by , 9 months ago
follow-up: 5 comment:4 by , 9 months ago
I believe that we should find a way to have db_default
behave the same way as generated fields when no value has been assigned to the instance when full_clean
is called (#35127). Requiring users to explicitly pass it to exclude
is simply bad ergonomics IMO.
One thing that the solution should take into account is that the value should only be skipped when the value is not assigned. In other words
class Article(models.Model): upvotes = models.PositiveIntegerField(db_default=0) a1 = Article(upvotes=-1) a1.full_clean() # should raise a ValidationError a2 = Article(upvotes=42) a2.full_clean() # should *not* raise a ValidationError a1 = Article() a1.full_clean() # should not raise a ValidationError
In other words, when a value is present it should be validated otherwise if db_default
is provided it should be assumed to be valid just like it's the case with generated fields.
A workaround in the mean time is to have both default
and db_default
assigned but obviously that kind of defeats the purpose of using db_default
in the first place.
comment:5 by , 9 months ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Bug → Cleanup/optimization |
Replying to Simon Charette:
I believe that we should find a way to have
db_default
behave the same way as generated fields when no value has been assigned to the instance whenfull_clean
is called (#35127). Requiring users to explicitly pass it toexclude
is simply bad ergonomics IMO.
Agreed, I think we should treat this as a cleanup not bug.
comment:6 by , 9 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:9 by , 9 months ago
Severity: | Normal → Release blocker |
---|
Bumping to a release blocker as Model.full_clean()
crashes on some expressions, e.g.
File "/django/tests/field_defaults/tests.py", line 175, in test_full_clean obj.full_clean() File "/django/django/db/models/base.py", line 1586, in full_clean self.clean_fields(exclude=exclude) File "/django/django/db/models/base.py", line 1641, in clean_fields setattr(self, f.attname, f.clean(raw_value, self)) File "/django/django/db/models/fields/__init__.py", line 830, in clean value = self.to_python(value) File "/django/django/db/models/fields/__init__.py", line 1620, in to_python parsed = parse_datetime(value) File "/django/django/utils/dateparse.py", line 114, in parse_datetime return datetime.datetime.fromisoformat(value) TypeError: fromisoformat: argument must be str
comment:10 by , 9 months ago
Bug in 414704e88d73dafbcfbb85f9bc54cb6111439d3.
comment:11 by , 9 months ago
Owner: | changed from | to
---|---|
Triage Stage: | Accepted → Ready for checkin |
db_default
is just a default value, we cannot always exclude it fromfull_clean()
. What about not valid values provided explicitly increate()
? What aboutNone
when field is non-nullable? etc.