Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#14223 closed (fixed)

Inconsistent exception raising on DB integrity errors

Reported by: ramiro Owned by: ramiro
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)

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 5 years ago.
Patch for this issue, includes tests
14223.2.diff (4.5 KB) - added by ramiro 5 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 4 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 5 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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

Changed 5 years ago by ramiro

Patch for this issue, includes tests

comment:2 Changed 5 years ago by ramiro

  • Has patch set
  • Owner changed from nobody to ramiro
  • Status changed from new to assigned

comment:3 Changed 5 years ago by ramiro

  • Description modified (diff)

Changed 5 years ago by ramiro

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

Changed 4 years ago by ramiro

comment:4 Changed 4 years ago by ramiro

  • Resolution set to fixed
  • Status changed from assigned to closed

(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 4 years ago by ramiro

(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