Attempt to create object with repeated value on a custom PK raises IntegrityError on wrong field

[Evandro Myller]

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.

[Evandro Myller]

full traceback

[Evandro Myller]

example models

[Tim Graham]

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.

[Baptiste Mispelon]

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.

[Tim Graham]

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.

[Evandro Myller]

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.

Thanks for looking into the issue.

[Evandro Myller]

admin module

[Evandro Myller]

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.

[fero]

I debugged it for a while during the Django sprint at pycon6 Italy and it seems to be quite a ​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:

1. one record with yyy already exists -> ValidationError should be raised and this could happen after fixing the validate_unique above
2. 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!

[Mariusz Felisiak]

#33512 was a duplicate, for creating a child instance with a manually assigned parent containing DateTimeField(auto_add_now=True).