[Django] #24539: Attempt to create object with repeated value on a custom PK raises IntegrityError on wrong field
Django
IntegrityError on wrong field
-------------------------------+--------------------
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
I have this model {{{Product}}}, child of an abstract model which defines
a {{{created}}} field with {{{auto_now_add}}} set. All the CRUD works just
fine so far.
The problem happens when I add a custom primary key to {{{Product}}}: If I
try to '''create''' a {{{Product}}} object '''through the admin''' giving
the {{{reference}}} field a repeated value, expecting to see a nice
validation error message saying that another object with that reference
already exists, I get an {{{IntegrityError}}} stating that the
{{{created}}} field cannot be {{{NULL}}} (traceback attached) -- nothing
even about the custom primary key.
Note: I noticed the exception is raised from an {{{UPDATE}}} query, which
is really odd since I'm posting data from the admin add view.
--
Ticket URL: <https://code.djangoproject.com/ticket/24539>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by emyller):
* Attachment "models.py" removed.
models example
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by emyller):
* Attachment "traceback.txt" added.
full traceback
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by emyller):
* Attachment "models.py" added.
example models
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by emyller):
* Attachment "models.py" added.
example models
--
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by emyller):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
> I have this model {{{Product}}}, child of an abstract model which defines
> a {{{created}}} field with {{{auto_now_add}}} set. All the CRUD works
> just fine so far.
>
> The problem happens when I add a custom primary key to {{{Product}}}: If
> I try to '''create''' a {{{Product}}} object '''through the admin'''
> giving the {{{reference}}} field a repeated value, expecting to see a
> nice validation error message saying that another object with that
> reference already exists, I get an {{{IntegrityError}}} stating that the
> {{{created}}} field cannot be {{{NULL}}} (traceback attached) -- nothing
> even about the custom primary key.
>
> Note: I noticed the exception is raised from an {{{UPDATE}}} query, which
> is really odd since I'm posting data from the admin add view.
New description:
I have this model {{{Product}}}, child of an abstract model which defines
a {{{created}}} field with {{{auto_now_add}}} set. All the CRUD works just
fine so far.
The problem happens when I add a custom primary key to {{{Product}}}: If I
try to '''create''' a {{{Product}}} object through the admin giving the
{{{reference}}} field a repeated value, expecting to see a nice validation
error message saying that another object with that reference already
exists, I get an {{{IntegrityError}}} stating that the {{{created}}} field
cannot be {{{NULL}}} (traceback attached) -- nothing even about the custom
primary key.
Notes:
- I noticed the exception is raised from an {{{UPDATE}}} query, which is
really odd since I'm posting data from the admin add view.
- {{{Product(reference=x).save()}}} raises the same exception, but
{{{Product.objects.create(reference=x}}} raises the expected exception
({{{IntegrityError}}} about the PK's UNIQUE constraint), which proves that
it's not a problem on the admin.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/24539#comment:1>
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
Comment:
I'm unable to reproduce the admin problem with the provided `models.py`
file. I see the "Product with this Reference already exists." error in the
admin as expected (tested on master and stable/1.7.x).
I could reproduce the `Product(reference=x).save()` problem though. Test
attached.
--
Ticket URL: <https://code.djangoproject.com/ticket/24539#comment:2>
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* Attachment "24539-test.diff" added.
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by bmispelon):
I'm not sure this is actually a bug.
When you save a mode that has its primary key set, Django will look in the
database to see if that key exists.
If it's found, then the corresponding record is updated (hence why you see
an `UPDATE` query) with the values set on the instance.
What's happening here is that the `UPDATE` query tries to set `created` to
`NULL` because the object you're saving has `created = None`.
--
Ticket URL: <https://code.djangoproject.com/ticket/24539#comment:3>
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Maybe not, I'm not sure the behavior is well defined (i.e. if should
expect `Field.pre_save()` to be called in such a situation). Probably not
very high priority at least. I'd say we can close this unless the reporter
can provide more details about how to reproduce the admin bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/24539#comment:4>
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by emyller):
Hi. I tried to isolate the code necessary to reproduce the bug through the
admin. Please check the attached file.
I agree it may be not high priority, but the raised exception is really
odd and should have some attention IMHO. I don't know the low-level
database-related modules in the Django package and currently don't have
time to study them well enough, otherwise I'd be happy to send a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/24539#comment:5>
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by emyller):
* Attachment "admin.py" added.
admin module
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by emyller):
I believe it's not an admin-related problem. The problem is gone if I
remove the `clean` method from the model form (or call
{{{super(ProductForm, self).clean()}}} on it). Still odd, but this might
bring in some clue.
--
Ticket URL: <https://code.djangoproject.com/ticket/24539#comment:6>
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by fero):
I debugged it for a while during the Django sprint at pycon6 Italy and it
seems to be quite ''a [https://en.wikipedia.org/wiki/Pandora%27s_box
Pandora's box]''.
I reviewed it with @apollo13 and here are the results, for a bug that
can't be solved today.
In our opinion there are some bugs actually:
1. regarding to the ORM low-level exception, the story is: '''''when a
user add a new instance from the Admin UI, he expects to INSERT a new
object''''' here is a candidate patch:
{{{
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -969,7 +969,11 @@ class ModelAdmin(BaseModelAdmin):
"""
Given a model instance save it to the database.
"""
- obj.save()
+ if not change:
+ obj.save(force_insert=True)
+ else:
+ obj.save()
+
def delete_model(self, request, obj):
"""
}}}
in this way the ORM performs an "INSERT" and raises the right
`IntegrityError` exception '''UNIQUE constraint failed:
base_product.reference''' (provided that you use python3 or you have
applied some fixes/patches from #23643)
2. regarding to the UX, the story is: '''''when a user input some wrong
values in a form, he expects to get a `ValidationError` with pink-rabbits
decorations'''''. I digged in this part with the help of @apollo13 and I
noticed that the overridden primary_key field is correctly validated with
the following "adjustment" ;) :
{{{
--- a/django/forms/models.py
+++ b/django/forms/models.py
@@ -433,7 +433,7 @@ class BaseModelForm(BaseForm):
self.instance = construct_instance(self, self.instance,
opts.fields, construct_instance_exclude)
try:
- self.instance.full_clean(exclude=exclude,
validate_unique=False)
+ self.instance.full_clean(exclude=exclude,
validate_unique=True)
except ValidationError as e:
self._update_errors(e)
}}}
Agreeing with @apollo13 this topic requires further discussion. So I leave
it up to you dear core developers ;) This regards to `ModelForm` though
and so has wider impact than the only admin interface (new ticket needed?)
3. after that, the update of a primary key value has another mess to deal
with: '''''when a user update the value, he expects to just update that
value ;)'''''.
The following cases could happen when value switches from `xxx` to `yyy`:
a. one record with `yyy` already exists -> `ValidationError` should be
raised and this could happen after fixing the `validate_unique` above
b. one record with `yyy` does not exists -> the value is updated.
Currently when the user update the value to `yyy`, the ORM performs
`UPDATE table SET k='yyy' WHERE k='yyy'` instead of `UPDATE table SET
k='yyy' WHERE k='xxx'`
That's my end of the sprint, it has been a good day, thanks Django team!
--
Ticket URL: <https://code.djangoproject.com/ticket/24539#comment:7>
Django
IntegrityError on wrong field
Reporter: emyller | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* component: Uncategorized => Database layer (models, ORM)
--
Ticket URL: <https://code.djangoproject.com/ticket/24539#comment:8>
Django
IntegrityError on wrong field
Reporter: Evandro Myller | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
#33512 was a duplicate, for creating a child instance with a manually
assigned parent containing `DateTimeField(auto_add_now=True)`.
--
Ticket URL: <https://code.djangoproject.com/ticket/24539#comment:9>