Database errors in the shell should roll back the transaction

[Glenn Maynard]

If an SQL statement fails in Postgresql, the connection refuses to run any further SQL commands until the transaction is rolled back. In normal operation, this is fine; but in the shell, it's a constant aggrevation.

This is related to #852, but that ticket was asking to automatically rollback inside the connection itself, which isn't good. This ticket is only regarding the shell. If I run a function that causes an SQL error, the shell's connection should not be left in an unusable state, forcing me to manually import db.transaction and roll back a transaction I never asked for. The shell should catch DatabaseError and rollback before returning to the console.

The attached patch implements this. I've only tested with Postgresql. This does not affect IPython (which I know nothing about).

Before:

>>> obj = User(name="Bill")
>>> obj.save()
...
IntegrityError: duplicate key value violates unique constraint "app_user_name_key"

>>> obj.name = "Jim"
>>> obj.save()
...
InternalError: current transaction is aborted, commands ignored until end of transaction block

Now:

>>> obj = User(name="Bill")
>>> obj.save()
...
IntegrityError: duplicate key value violates unique constraint "app_user_name_key"

>>> obj.name = "Jim"
>>> obj.save()
>>>

[fnl]

Any chance this issue will be 'fixed' in 1.2? This is a _very_ annoying problem.

[Glenn Maynard]

I hope so. I'm sure most every Postgresql user of Django has been bitten by this.

[Adrian Holovaty]

I agree we should fix this, but I don't think the attached patch is the right solution. It appears to affect only manage.py shell, and a more bulletproof solution would affect any shell, including IPython and the normal python shell.

[Glenn Maynard]

In my opinion, this is ultimately a bug in Postgresql: errors should *not* prevent subsequent commands in a transaction from working. The only generic way I know to work around this is to set a savepoint before each command and commit it immediately after, so if an error occurs you can roll back to the checkpoint to unset the error flag. The overhead of that isn't acceptable in production, of course, and doing that only when in a shell (which I'm not even sure is possible) would be a debugging nightmare.

The correct fix is to convince Postgresql upstream to add a flag to disable this behavior, so errors within a transaction don't abort the entire transaction. This would make Postgresql's behavior match other SQL engines. I've had trouble with this many times over the years and have never had any luck getting them to fix this.

[gdufferin@…]

This bug also persists when creating a custom management task i.e /manage.py mycustomtask. Obviously, it would be ideal for database errors not to occur, thus not needing the rollback, but in some cases this is unavoidable, i.e in a try: Model.objects.get(*params) except: do something else...scenario.

Is there a fix?

[Wolph]

Replying to Glenn:

In my opinion, this is ultimately a bug in Postgresql: errors should *not* prevent subsequent commands in a transaction from working. The only generic way I know to work around this is to set a savepoint before each command and commit it immediately after, so if an error occurs you can roll back to the checkpoint to unset the error flag. The overhead of that isn't acceptable in production, of course, and doing that only when in a shell (which I'm not even sure is possible) would be a debugging nightmare.

Can you explain the rationale behind that? If _anything_ inside my transaction gives me an error which I had not anticipated (otherwise I should have used a savepoint) it should just rollback the entire transaction. Just allowing new commands like nothing has happened can easily cause data loss and goes directly against ACID compliance.

The correct fix is to convince Postgresql upstream to add a flag to disable this behavior, so errors within a transaction don't abort the entire transaction. This would make Postgresql's behavior match other SQL engines. I've had trouble with this many times over the years and have never had any luck getting them to fix this.

No, the correct fix is to make Django default to AutoCommit settings with Postgres. If you want to continue running commands after your transaction has died than you shouldn't be using transactions in the first place.

My proposed fix, make autocommit the default setting for postgres.

[Glenn Maynard]

Can you explain the rationale behind that? If _anything_ inside my transaction gives me an error which I had not anticipated (otherwise I should have used a savepoint) it should just rollback the entire transaction. Just allowing new commands like nothing has happened can easily cause data loss and goes directly against ACID compliance.

Nothing says that errors are unexpected. If an unexpected error does happen, it's the application's job to roll back the transaction; it's *not* the database's job to *force* it to.

No, the correct fix is to make Django default to AutoCommit? settings with Postgres. If you want to continue running commands after your transaction has died than you shouldn't be using transactions in the first place.

That doesn't make sense. Of course I should be using transactions.

My proposed fix, make autocommit the default setting for postgres.

The default behavior should not vary across databases.

I don't intend to debate this further, since this bug has sat around for well over three years (despite having a reasonable fix available for at least the most common and annoying problem), giving me little expectation of it getting fixed, and I'm not even using Django for anything right now.

[Wolph]

Replying to Glenn:

Nothing says that errors are unexpected. If an unexpected error does happen, it's the application's job to roll back the transaction; it's *not* the database's job to *force* it to.

Regardless, when the database gives you an error like this it can be impossible (e.g. with stored procedures, triggers, etc..) to fully understand what has happened behind the scenes.

The only reasonable cause of action in such cases is rolling back (either to a savepoint or the entire commit). There can be a lot of ambiguity if the database would allow you to continue after something like that. PostgreSQL has way too much features behind the scenes (rules, triggers, stored procedures) to allow you to fully know what is going on from Django.

If ambiguity is possible than there should be no option to commit after all. It can severely corrupt your database.

No, the correct fix is to make Django default to AutoCommit? settings with Postgres. If you want to continue running commands after your transaction has died than you shouldn't be using transactions in the first place.

That doesn't make sense. Of course I should be using transactions.

The principle of transactions is that you are sure that _all_ of your statements either commit succesfully or rollback. Not doing either of those goes against ACID.

My proposed fix, make autocommit the default setting for postgres.

The default behavior should not vary across databases.

But the default behaviour _is_ different. In MySQL the behaviour is very similar to autocommit mode by default. With MyISAM you don't even have transactions as an option. And having autocommit enabled doesn't stop you from manually opening a transaction like you would normally do.

[Aymeric Augustin <aymeric.augustin@…>]

In 14cddf51c5f001bb426ce7f7a83fdc52c8d8aee9:

Merged branch 'database-level-autocommit'.

Fixed #2227: atomic supports nesting.
Fixed #6623: commit_manually is deprecated and atomic doesn't suffer from this defect.
Fixed #8320: the problem wasn't identified, but the legacy transaction management is deprecated.
Fixed #10744: the problem wasn't identified, but the legacy transaction management is deprecated.
Fixed #10813: since autocommit is enabled, it isn't necessary to rollback after errors any more.
Fixed #13742: savepoints are now implemented for SQLite.
Fixed #13870: transaction management in long running processes isn't a problem any more, and it's documented.
Fixed #14970: while it digresses on transaction management, this ticket essentially asks for autocommit on PostgreSQL.
Fixed #15694: atomic supports nesting.
Fixed #17887: autocommit makes it impossible for a connection to stay "idle of transaction".