#30581 closed New feature (fixed)
Allow constraints to be used for validation (in Python)
Reported by: | Carlton Gibson | Owned by: | Gagaro |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 4.0 |
Severity: | Normal | Keywords: | constraints, validation |
Cc: | Simon Charette, Adam Johnson, Ian Foote, Fabio Caritas Barrionuevo da Luz, Gagaro | 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
Follow-up from #30547, where we documented how database constraints relate to validation.
Executive summary: In general they don't. Instead you get an IntegrityError
on save()
. Except, UniqueConstraint
is able to tie-in to the existing validate_unique()
logic, so you will get a ValidationError
on full_clean()
.
It would be nice if all constraints could (in principle) be used for Python-level validation, in addition to setting up the database constraint. (We might image ModelForm
, say, being able to extract correct validation logic from the model definition, and so on.)
Initial thought is that Python validators could be inferred from Q
objects:
CheckConstraint(check=Q(age__gte=18), name='age_gte_18')
Looks like it maps to a simple lambda age: age > 18
, for example.
More complex examples will cause issues. Discussion:
So we might be able to do some auto-generation but first goal here should (I guess) be an optional hook on BaseConstraint
that if implemented can be picked up for use in validation. (Or similar.)
I'll create this as Accepted, since existing discussions imply that.
Change History (31)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
Well, feel free, but read those comments. :)
If it were me, I’d come up with a half idea and then post to the DevelopersMailingList
to get input from others, before coding too much... — but yes, it just needs someone to try I’d guess.
comment:3 by , 5 years ago
Ok, I appreciate advice! I'll put together some thoughts and get feedback before assigning to myself or writing too much code (or figuring out its beyond my knowledge level). Gracias.
Ah ok, on closer examination I see what you're saying. I didn't fully grasp the extent of the ambitions :)
comment:4 by , 5 years ago
FWIW I think the validation should be pushed to the database as much as possible. Trying to emulate constraint criterias in Python will hard if not impossible to implement in a correct way for all backends.
I'm not against an overridable hook but it should default to querying the database (e.g. 'SELECT %s > %s', (age, 18)
).
comment:5 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 5 years ago
I went through the previous pull request comments. As far as I could understand, generating some sort of validation from a Q object seems complicated, and there would arise a lot of issues. Does anyone have any recommendations?
comment:7 by , 5 years ago
Cc: | added |
---|
I suggest you have a look at the existing Model.validate_unique
logic and draft an API that would allow users to define their own constraints with custom validation.
The interface should be flexible enough to implement concepts such as unique_for_date
(and friends) and exclusion constraints in a way that is similar to how Field.unique=True
and Model._meta.unique_together
are handled right now.
I believe that we should aim for a setup where all of the current unique validation logic is deferred to UniqueConstraint
by making Field.unique=True
and unique_together
result in UniqueConstraint.auto_created = True
entries in Model._meta.constraints
. These auto_created
constraints entries would be ignored by migrations, just like ManyToManyField
auto-created intermediary models are, and should prove that the constraint enforcing mechanism is flexible enough to cover all of the current use cases validate_unique
currently has.
I order to achieve that we'll likely want to define a new Constraint.enforce
(or validate
) method that accepts an optional existing model instance and a set of excluded fields that would raise on violation. It would then be the responsibility of the subclass to implement or not this function (e.g. we could skip it on UniqueConstraint
with a condition
at first as they are more complex to implement).
I hope this gives you a better idea of a plan to tackle this issue. Happy to move the discussion to the mailing list to gather more feedback if deemed more appropriate, that's just the way I envisioned it.
comment:8 by , 5 years ago
Thanks for this informative reply. I’ll be sure to look into these and come up with some sort of a plan, to be sent to the mailing list. I am quite new (this is my first ticket), so I hope you guys will bear with me if I say something stupid.
comment:9 by , 5 years ago
I have gone through the issue and the related source code. I think I have a fair idea of what's causing the issue. What I dont understand is that what this ticket aims to achieve. Is there supposed to be a solution, to raise an error upon CheckConstraint
? Or is it supposed to be a custom validation method in BaseConstraint
, which lets users help define custom validation? What exactly is the Constraint.enforce
method supposed to do? It would be nice if I could get a bit more help with some examples. I am sorry, that I am having hard time understanding this, as I am a beginner.
comment:10 by , 5 years ago
Sanskar Jaiswal, to be honest I think comment:7 provides a pretty good picture of what ought to be done without baking in too many implementation details.
This ticket might be bit too tricky for a first as it requires good knowledge of the ORM, model validation, and database constraints. I suggest you have a look at other ones if you don't feel comfortable handling this one right now.
comment:11 by , 5 years ago
FWIW I started exploring the above idea and published my changes which pass most of the test suite but still require a bit of polishing.
In the light of the current barrage of issues related to checks that are not UniqueConstraint
aware I think the Constraint.auto_created
creation by Field.unique
and Model.Meta.unique_together
has merits as it would allow all of the unicity checks to be performed against _meta.constraints
(or the newly added total_unique_constraints
) instead of combining the three APIs all over the place.
comment:12 by , 5 years ago
Cc: | added |
---|
comment:13 by , 5 years ago
Cc: | added |
---|
comment:14 by , 4 years ago
Cc: | added |
---|
comment:15 by , 3 years ago
Cc: | added |
---|---|
Owner: | changed from | to
This issue is being discussed on the [mailing list](https://groups.google.com/g/django-developers/c/iiOnxAn3vy4/m/ZA2aPyWoBQAJ) and implementation has started.
comment:16 by , 3 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Needs tests: | set |
Patch needs improvement: | set |
Version: | 2.2 → 4.0 |
comment:22 by , 3 years ago
Needs documentation: | unset |
---|
comment:23 by , 3 years ago
Patch needs improvement: | unset |
---|
comment:24 by , 3 years ago
Needs tests: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
@carltongibson Is this something I can take on? Or do we need more design discussions?