Transaction problems with save(force_insert=True) on postgresql (and likely other transactional dbs)

[Richard Davies <richard.davies@…>]

I attach a patch which adds an extra test, regressiontests/save_force_insert. This test succeeds with a SQLite backend, but fails with a postgreSQL backend. The postgreSQL error cites the line immediately after the call to save(force_insert=True), but I believe that this means that the transaction was aborted when save(force_insert=True) threw an exception, and that some form of savepoint protection may be needed inside the implementation of save().

$ ./runtests.py --settings=sqlite save_force_insert
----------------------------------------------------------------------
Ran 1 test in 0.005s

OK


$ ./runtests.py --settings=postgresql_psycopg2 save_force_insert
======================================================================
FAIL: Doctest: regressiontests.save_force_insert.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/elastic/django-dev/trunk/django/test/_doctest.py", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for regressiontests.save_force_insert.models.__test__.API_TESTS
  File "/home/elastic/django-dev/trunk/tests/regressiontests/save_force_insert/models.py", line unknown line number, in API_TESTS

----------------------------------------------------------------------
File "/home/elastic/django-dev/trunk/tests/regressiontests/save_force_insert/models.py", line ?, in regressiontests.save_force_insert.models.__test__.API_TESTS
Failed example:
    ManualPrimaryKeyTest.objects.get(id=1).data == 'Original'
Exception raised:
    Traceback (most recent call last):
      File "/home/elastic/django-dev/trunk/django/test/_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest regressiontests.save_force_insert.models.__test__.API_TESTS[3]>", line 1, in <module>
        ManualPrimaryKeyTest.objects.get(id=1).data == 'Original'
      File "/home/elastic/django-dev/trunk/django/db/models/manager.py", line 81, in get
        return self.get_query_set().get(*args, **kwargs)
      File "/home/elastic/django-dev/trunk/django/db/models/query.py", line 296, in get
        num = len(clone)
      File "/home/elastic/django-dev/trunk/django/db/models/query.py", line 152, in __len__
        self._result_cache = list(self.iterator())
      File "/home/elastic/django-dev/trunk/django/db/models/query.py", line 267, in iterator
        for row in self.query.results_iter():
      File "/home/elastic/django-dev/trunk/django/db/models/sql/query.py", line 206, in results_iter
        for rows in self.execute_sql(MULTI):
      File "/home/elastic/django-dev/trunk/django/db/models/sql/query.py", line 1665, in execute_sql
        cursor.execute(sql, params)
    InternalError: current transaction is aborted, commands ignored until end of transaction block



----------------------------------------------------------------------
Ran 1 test in 0.012s

FAILED (failures=1)

[Richard Davies <richard.davies@…>]

Extra test case (note that the init.py must be created too)

[Malcolm Tredinnick]

The save() method won't do transaction management for you. If save() throws an IntegrityError, it's up to your code to deal with it. That's why force_insert are to be used extremely rarely (and why, for example, create() does the savepoint management).

So if I understand the ticket correctly -- which is that after raising an IntegrityError the tests must clean up and rollback -- the answer is, yes, that's correct. Intended behaviour.

If there's some other problem you're reporting here that I'm missing, please reopen with an alternate explanation. But Django isn't going to do your transaction management for you, since there's never a single correct solution.

[Malcolm Tredinnick]

Err ... when I wrote create() above, I meant get_or_create().

[Richard Davies <richard.davies@…>]

Agreed with Malcolm that this ticket was a mistake. Before I sign off, here's a test case for force_insert functionality only, which _does_ do the transaction management itself, and succeeds on both databases

[Malcolm Tredinnick]

As best I can tell, this test is duplicating one of the cases in tests/force_insert_update/models.py.

[Jacob]

Milestone 1.0 deleted