#12881 closed (fixed)
Unique constraint error with model inheritance while ModelForm should not validate
Reported by: | fgaudin | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.1 |
Severity: | Keywords: | unique constraint, ModelForm, inheritance | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The use case is :
- ParentClass has an attribute "name" with unique=True
- FirstSubClass inherits ParentClass
- SecondSubClass inherits ParentClass
With a ModelForm for each subclass, I can't create two FirstSubClasses with the same name, neither two SecondSubClasses, the form doesn't validate, what is expected.
But if I create a FirstSubClass instance and try to create a SecondSubClass instance, the form validate and unique constraint error is raised by postgres.
A project showing the use case with unit tests is in attachment.
Attachments (2)
Change History (13)
by , 15 years ago
Attachment: | django_bug.tar.gz added |
---|
comment:2 by , 15 years ago
Component: | Forms → Database layer (models, ORM) |
---|---|
milestone: | → 1.2 |
Triage Stage: | Unreviewed → Accepted |
Ugh. The model class used for the unique checks is the model class associated with the form. In the case where this class is derived from another, the resulting query involves a join of the tables for the derived and parent models. Thus the unique check is improperly constraining the unique test in the case where the unique constraint is declared in the parent class. I think the unique check needs to be made using the model class where the unique constraint actually came from. I'll attach a patch that works for the posted test, but I'm not too familiar with all the model class internals so there well be a more straightforward way to determine the right class to use for the check.
comment:3 by , 15 years ago
Has patch: | set |
---|---|
Needs tests: | set |
comment:4 by , 15 years ago
#13079 reports an additional problem with unique_together
that persists even after a fix for this is applied. Apparently the unique_together
constraint on the parent is not even seen as something that needs checking for a ModelForm for the child model.
comment:5 by , 15 years ago
Patch needs improvement: | set |
---|
Given the report in #13079 I think the approach in the patch here is wrong. Unique constraints on a field are only being seen because all the fields are pushed down to the child model. #13079 shows unique_together
specified on a parent is invisible to the child model, thus the logic that gathers the unique checks in the first place needs to be changed. I think it would better for that logic to keep track of the model that is the source of the each unique constraint, rather than later on attempting to figure out which model should be used for performing the check.
comment:6 by , 15 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
I've attached a new patch, with tests, that attempts to fix the issues for inherited unique, unique_together, and unique for dates. Review from someone with a clue in the ORM area would be helpful.
comment:7 by , 15 years ago
Patch needs improvement: | set |
---|
Full test suite fails with various formset unique validation errors so there's clearly something not quite right with it yet.
by , 15 years ago
Attachment: | 12881.diff added |
---|
comment:8 by , 15 years ago
Patch needs improvement: | unset |
---|
New patch, passes full test suite. Both model formsets and some validation tests depend on what is returned by _get_unique_checks()
, so they were unexpectedly (to me) broken by changing what that returned.
comment:9 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:10 by , 15 years ago
Though this problem afflicts 1.1.X as well, backporting the fix is not trivial. Subsequent to 1.1 all the unique validation logic has been moved from the ModelForm code into the base Model code. Moving the logic of this fix (which relies on internals of model data structures) back out to the ModelForm class is decidedly ugly, so at present I'm inclined to leave this as a 1.2-only fix.
project which shows the validation behaviour of ModelForm in case of inheritance