get_or_create() raises "DatabaseError: no such savepoint" instead of IntegrityError (PostgreSQL)

[Antti Kaihola]

Given the following models in a world app:

class Country(models.Model):
    name = models.CharField(max_length=50)

class CountryExtraInfo(models.Model):
    country = models.ForeignKey(Country, primary_key=True)
    info = models.TextField(blank=True, default='')

and assuming no Countries have been created yet, the following happens:

>>> from world.models import CountryExtraInfo
>>> CountryExtraInfo.objects.get_or_create(pk=1)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "django/db/models/manager.py", line 135, in get_or_create
    return self.get_query_set().get_or_create(**kwargs)
  File "django/db/models/query.py", line 389, in get_or_create
    transaction.savepoint_rollback(sid, using=self.db)
  File "django/db/transaction.py", line 235, in savepoint_rollback
    connection._savepoint_rollback(sid)
  File "django/db/backends/__init__.py", line 70, in _savepoint_rollback
    self.cursor().execute(self.ops.savepoint_rollback_sql(sid))
  File "django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
DatabaseError: no such savepoint

Also:

>>> from world.models import Country, CountryExtraInfo
>>> c = Country()
>>> c.id = 1
>>> CountryExtraInfo.objects.get_or_create(country=c)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "django/db/models/manager.py", line 135, in get_or_create
    return self.get_query_set().get_or_create(**kwargs)
  File "django/db/models/query.py", line 389, in get_or_create
    transaction.savepoint_rollback(sid, using=self.db)
  File "django/db/transaction.py", line 235, in savepoint_rollback
    connection._savepoint_rollback(sid)
  File "django/db/backends/__init__.py", line 70, in _savepoint_rollback
    self.cursor().execute(self.ops.savepoint_rollback_sql(sid))
  File "django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
DatabaseError: no such savepoint

In these situations, it would make more sense to get an IntegrityError instead.

Test environment:

• Python 2.6.6
• Django r15242
• PostgreSQL 8.4.6
• psycopg2 2.2.1

[Russell Keith-Magee]

Agreed; a savepoint error isn't the right response here. I'm not completely convinced that IntegrityError is right, either; get_or_create should be able to... well... get, or create. However, I can see that there are issues with that in practice.

[Dougal Matthews]

I've attached a patch with a failing test case to show the problem.

The problem here is only seen (from what I can tell and test) in ORM backends that support savepoints. Basically, when the integrity error is raised a rollback is attempted to the save point, however, it doesn't exist thus raising the "DatabaseError: no such savepoint". I can't figure out why the save point is lost, would it be rolled back automatically when the integrity error happens?

The following lines then in get_or_create actually re-raise the Integrity error and I think this should be the expected behaviour.

[Dougal Matthews]

Another observation while I worked on this ticket: get_or_create has the following lines.

            except IntegrityError, e:
                transaction.savepoint_rollback(sid, using=self.db)
                try:
                    return self.get(**lookup), False
                except self.model.DoesNotExist:
                    raise e

Would it not be preferable to do the following to preserve the full original traceback? (Note the addition of a pass and then using "raise" rather than "raise e")

            except IntegrityError, e:
                transaction.savepoint_rollback(sid, using=self.db)
                try:
                    return self.get(**lookup), False
                except self.model.DoesNotExist:
                    pass
                raise

This is not directly related to the tickets issue, but to the more general problem of improving get_or_creates error reporting.

[Dougal Matthews]

I turned on the log in postgres to see what was happening in the SQL.

2011-06-25 03:03:12 PDT LOG:  statement: SELECT "get_or_create_integrityerrortest"."country_id", "get_or_create_integrityerrortest"."info" FROM "get_or_create_integrityerrortest" WHERE "get_or_create_integrityerrortest"."country_id" = 1 
2011-06-25 03:03:12 PDT LOG:  statement: SAVEPOINT s1217009984_x1
2011-06-25 03:03:12 PDT LOG:  statement: INSERT INTO "get_or_create_integrityerrortest" ("country_id", "info") VALUES (1, E'')
2011-06-25 03:03:12 PDT LOG:  statement: COMMIT
2011-06-25 03:03:12 PDT ERROR:  insert or update on table "get_or_create_integrityerrortest" violates foreign key constraint "get_or_create_integrityerrortest_country_id_fkey"
2011-06-25 03:03:12 PDT DETAIL:  Key (country_id)=(1) is not present in table "get_or_create_person".
2011-06-25 03:03:12 PDT STATEMENT:  COMMIT
2011-06-25 03:03:12 PDT LOG:  statement: BEGIN; SET TRANSACTION ISOLATION LEVEL READ COMMITTED
2011-06-25 03:03:12 PDT LOG:  statement: ROLLBACK TO SAVEPOINT s1217009984_x1
2011-06-25 03:03:12 PDT ERROR:  no such savepoint
2011-06-25 03:03:12 PDT STATEMENT:  ROLLBACK TO SAVEPOINT s1217009984_x1
2011-06-25 03:03:12 PDT LOG:  statement: ROLLBACK

It appears that postgres has gotten rid of the savepoint.

[Andrew Godwin]

So, I worked with Dougal on this at the Europython sprints, and the problem is that the code tries to COMMIT the previous transaction, which fails, and then tries to ROLLBACK TO SAVEPOINT after the commit has failed.

As far as I can tell, the failing COMMIT implicitly clears the savepoints in the previous transaction. The postgres client libraries reflect this by sending a BEGIN along with the ROLLBACK TO SAVEPOINT, as they assume the failing COMMIT cancels the previous transaction. This then causes the ROLLBACK TO SAVEPOINT to fail, as it's a new transaction without that savepoint.

We changed the backend to not send the BEGIN after the COMMIT, but then Postgres complained that this was invalid, and so I suspect it's not valid to send ROLLBACK TO SAVEPOINT _after_ COMMIT (all the examples in the postgres docs have it before a COMMIT).

It seems, then, that the "clean" fix would be to make this code just start a new transaction if its COMMIT fails, but that leads to potential nesting errors depending if the outer code has already started a transaction or not. I can't see a nice way to roll back to the savepoint at the moment, but my brain hurts after too much poking around with transactions, and I can't find the part of the SQL spec that Postgres is apparently adhering to in this regard (I think it's in SQL 1999, and I can't find where to download it).

[Jacob]

Milestone 1.4 deleted

[anonymous]

What to do when in this situation where all get_or_create fail ?

TYIA :)

[Aymeric Augustin]

I just introduced django.db.transaction.atomic that precisely designed for this use case.

It uses "release savepoint" instead of "commit", which allows rolling back to the savepoint if necessary.

[Aymeric Augustin]

Indeed, this specific bug is fixed in master. I'm going to commit a variant of the test case attached by d0ugal.

The code provided by the reporter now raises an IntegrityError when the transaction is committed (immediately if no transaction is in progress) on databases that enforce integrity. It inserts bad data silently on other databases.

[Aymeric Augustin <aymeric.augustin@…>]

In 3a4276ffc35326f20e95890b50a67aebeabc9ad0:

Tested that get_or_create raises IntegrityError.

It used to raise "DatabaseError: no such savepoint" with the old
transaction management. Closes #15117.