Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#14223 closed (fixed)

Inconsistent exception raising on DB integrity errors

Reported by: Ramiro Morales Owned by: Ramiro Morales
Component: Database layer (models, ORM) Version: master
Severity: Keywords: integrityerror backends orm
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by Ramiro Morales)

It seems we aren't capturing the <db api driver module>.IntegrityError exceptions raised when connection.commit() is executed in the same way we do in execute().

Found this while creating tests to verify FK constraint in sqlite (#14204). My assertRaises(django.db.[utils.]IntegrityError, ...) weren't being triggered and printing the exception type shows <class 'pysqlite2.dbapi2.IntegrityError'>. Same thing happens now with postgres (see below).

The DatabaseError and IntegrityError unification was introduced in r12352, if this is confirmed as an issue it wouldn't be a regression in 1.2, but a 1.2 feature introduced in an incomplete fashion.

The two files pasted at the end form a small test case. When run under postgres you see:

======================================================================
ERROR: Try to create a model instance that violates a FK constraint. Should fail
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/r/django/sqlite_fk2/tests/regressiontests/fk_constraints/tests.py", line 22, in test_integrity_checks_on_creation
    a.save()
  File "/home/r/django/sqlite_fk2/django/db/models/base.py", line 434, in save
    self.save_base(using=using, force_insert=force_insert, force_update=force_update)
  File "/home/r/django/sqlite_fk2/django/db/models/base.py", line 534, in save_base
    transaction.commit_unless_managed(using=using)
  File "/home/r/django/sqlite_fk2/django/db/transaction.py", line 175, in commit_unless_managed
    connection._commit()
  File "/home/r/django/sqlite_fk2/django/db/backends/__init__.py", line 32, in _commit
    return self.connection.commit()
IntegrityError: insert or update on table "fk_constraints_article" violates foreign key constraint "fk_constraints_article_reporter_id_fkey"
DETAIL:  Key (reporter_id)=(30) is not present in table "fk_constraints_reporter".

<class 'psycopg2.IntegrityError'>
# models.py
from django.db import models

class Reporter(models.Model):
    first_name = models.CharField(max_length=30)
    last_name = models.CharField(max_length=30)
    email = models.EmailField()

    def __unicode__(self):
        return u"%s %s" % (self.first_name, self.last_name)

class Article(models.Model):
    headline = models.CharField(max_length=100)
    pub_date = models.DateField()
    reporter = models.ForeignKey(Reporter)

    def __unicode__(self):
        return self.headline

    class Meta:
        ordering = ('headline',)
# tests.py
from datetime import datetime

from django.db import utils
from django.test import TransactionTestCase

from models import Reporter, Article

import sys

class FkConstraintsTest(TransactionTestCase):

    def setUp(self):
        # Create a Reporter.
        self.r = Reporter.objects.create(first_name='John', last_name='Smith', email='john@example.com')
        #self.r2 = Reporterobjects.create(first_name='Paul', last_name='Jones', email='paul@example.com')

    def test_integrity_checks_on_creation(self):
        """Try to create a model instance that violates a FK constraint. Should fail"""
        a = Article(headline="This is a test", pub_date=datetime(2005, 7, 27), reporter_id=30)
        #self.assertRaises(utils.IntegrityError, a.save)
        try:
            a.save()
        except Exception, e:
            print >>sys.stderr, type(e)
            raise

    def test_integrity_checks_on_update(self):
        """Try to update a model instance introducing a FK constraint violation. Should fail"""
        # Create an Article.
        Article.objects.create(headline="Test article", pub_date=datetime(2010, 9, 4), reporter=self.r)
        # Retrive it from the DB
        a = Article.objects.get(headline="Test article")
        a.reporter_id = 30
        #self.assertRaises(utils.IntegrityError, a.save)
        try:
            a.save()
        except Exception, e:
            print >>sys.stderr, type(e)
            raise

Attachments (3)

14223.1.diff (4.0 KB) - added by Ramiro Morales 6 years ago.
Patch for this issue, includes tests
14223.2.diff (4.5 KB) - added by Ramiro Morales 6 years ago.
Enhanced patch, check for django.db.IntegrityError instead of django.db.utils IntegrityError in the tests, added a comment
14223-r14315.diff (4.9 KB) - added by Ramiro Morales 6 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 6 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

IMO we should capture and convert ALWAYS, it just seems like the sane thing to do.

Changed 6 years ago by Ramiro Morales

Attachment: 14223.1.diff added

Patch for this issue, includes tests

comment:2 Changed 6 years ago by Ramiro Morales

Has patch: set
Owner: changed from nobody to Ramiro Morales
Status: newassigned

comment:3 Changed 6 years ago by Ramiro Morales

Description: modified (diff)

Changed 6 years ago by Ramiro Morales

Attachment: 14223.2.diff added

Enhanced patch, check for django.db.IntegrityError instead of django.db.utils IntegrityError in the tests, added a comment

Changed 6 years ago by Ramiro Morales

Attachment: 14223-r14315.diff added

comment:4 Changed 6 years ago by Ramiro Morales

Resolution: fixed
Status: assignedclosed

(In [14320]) Fixed #14223 -- Extended unification of exception raised in presence of integrity constraint violations.

The unification had been introduced in r12352 and native backend exceptions still
slipped through in cases that end in connection.commit() call. Thanks Alex,
Jacob and Carl for reviewing.

comment:5 Changed 6 years ago by Ramiro Morales

(In [14321]) [1.2.X] Fixed #14223 -- Extended unification of exception raised in presence of integrity constraint violations.

The unification had been introduced in r12352 and native backend exceptions still
slipped through in cases that end in connection.commit() call. Thanks Alex,
Jacob and Carl for reviewing.

Backport of [14320] from trunk

Note: See TracTickets for help on using tickets.
Back to Top