Peewee: Incompleted transactions with PostgreSQL

[rudyryk]

How to deal with incompleted transactions in PostgreSQL? It seems we can't make queries after single transaction fail. Instead we get: psycopg2.InternalError: current transaction is aborted, commands ignored until end of transaction block

Here's minimal code sample to reproduce the issue:

import peewee
db = peewee.PostgresqlDatabase(
            database='test',
            host='localhost',
            port=None,
            user='test',
            password=None)

class User(peewee.Model):
   username = peewee.CharField(max_length=50, unique=True)

   class Meta:
       database = db

User.create_table()
User.create(username='test')

So, I get single user with username "test". If I try to create another one, I'll get an error:

>>> User.create(username='test')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 2193, in create
    inst.save(force_insert=True)
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 2278, in save
    new_pk = insert.execute()
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 1623, in execute
    return self.database.last_insert_id(self._execute(), self.model_class)
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 1314, in _execute
    return self.database.execute_sql(sql, params, self.require_commit)
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 1721, in execute_sql
    res = cursor.execute(sql, params or ())
psycopg2.IntegrityError: duplicate key value violates unique constraint "user_username"
DETAIL:  Key (username)=(test) already exists.

And that's predictable and fine! (By the way, how should we catch this kind of exception?)

But when I try to make another request, it fails:

>>> User.get(username='test')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 2203, in get
    return sq.get()
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 1521, in get
    return clone.execute().next()
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 1556, in execute
    self._qr = ResultWrapper(self.model_class, self._execute(), query_meta)
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 1314, in _execute
    return self.database.execute_sql(sql, params, self.require_commit)
  File "/Users/test/.virtualenvs/test/lib/python3.3/site-packages/peewee.py", line 1721, in execute_sql
    res = cursor.execute(sql, params or ())
psycopg2.InternalError: current transaction is aborted, commands ignored until end of transaction block

And I need to drop database connection and create a new one to make it work again.

[rudyryk]

Sorry, I missed a couple of updates :( After updating works great! :+1:

[rudyryk]

No, bug is still present in 2.1.4 and in current development version with Python 3.3 at least. (Last time I just reproduced it incorrectly).

[coleifer]

You will need to issue a rollback, if you are using the database in autocommit mode:

User.create(username='test')
try:
    User.create(username='test')  # error
except:
    db.rollback()
# Continue as normal.

[rudyryk]

Sure, that's workaround. It seems that I should wrap every save / update / create into try..except block and that's not really good. I think rollback by default would be correct for autocommit mode. Am I missing something?

[rudyryk]

Well, I think it's developer's responsibility to handle application level errors e.g.:

try:
    User.create(username='test')  # error
except User.ValidationError:
    pass # do something

And I think database level errors should be handled under the hood in 'autopilot' mode - which is 'autocommit' mode.

Does that sound reasonable? :)

[rudyryk]

I think I can write an implementation and I'd want to work out good design solution :)

Another smaller problem I faced with explicit rollback is that I have to import global database db object:

User.create(username='test')
try:
    User.create(username='test')  # error
except:
    db.rollback()

I think Model.get_database() method would be nicer.

[coleifer]

Sure, that's workaround.

In my opinion it is not a workaround -- if you do not handle transactions or application errors properly peewee won't try to figure out how to fix it for you.

There are a number of transaction helpers that can help you avoid the "wrap every query" part:

• http://peewee.readthedocs.org/en/latest/peewee/cookbook.html#working-with-transactions
• http://peewee.readthedocs.org/en/latest/peewee/api.html#Database.commit_on_success

[rudyryk]

Ok, I see the point. My suggestion is consider autocommit mode like "I don't want to know about transactions at all".

[coleifer]

Right, and that's how it works, but if you trigger an IntegrityError in postgres it will mess up your implied transaction and you will have to roll it back yourself. You could subclass PostgresqlDatabase and just roll back if you encounter an error but I wouldn't recommend it.

Closing this as it is expected behavior.

[rudyryk]

Ok, then :) May be this should be highlighted somewhere is docs. I didn't actually expect that common single wrong save call may lock the whole database connection.

[tconkling]

Just want to chime in that this is unexpected to me as well.

@coleifer, in response to

My suggestion is consider autocommit mode like "I don't want to know about transactions at all".

you say,

Right, and that's how it works, but if you trigger an IntegrityError in postgres it will mess up your implied transaction and you will have to roll it back yourself.

But the whole idea of a having to rollback an "implied transaction" seems at odds with the stated notion of "I don't want to know about transactions."

I don't see a circumstance where code with autocommit=True would ever _not_ want an automatic rollback. Am I missing something?

[coleifer]

My expectation of what is "reasonable" behavior for these connections is
based on my experience using Django, which may be the root of the issue
here, since this is how Django treats integrity errors with psycopg2.

I agree, it is weird to roll back a transaction you never opened because
you encountered an error. At this point I'm not sure how many folks out
there have written code to handle the rollback() call, but I'm open to the
idea of rolling back automatically.

On Fri, Jan 10, 2014 at 7:07 PM, Tim Conkling [email protected]:

Just want to chime in that this is unexpected to me as well.

@coleifer https://github.com/coleifer, in response to

My suggestion is consider autocommit mode like "I don't want to know about
transactions at all".

you say,

Right, and that's how it works, but if you trigger an IntegrityError in
postgres it will mess up your implied transaction and you will have to roll
it back yourself.

But the whole idea of a having to rollback an "implied transaction" seems
at odds with the stated notion of "I don't want to know about transactions."

I don't see a circumstance where code with autocommit=True would ever
_not_ want an automatic rollback. Am I missing something?

—
Reply to this email directly or view it on GitHubhttps://github.com/coleifer/peewee/issues/240#issuecomment-32081866
.

[tconkling]

As a new user of peewee (which is great - thanks for creating & maintaining it!), I'm very much in favor of changing the default; the current behavior was confusing to me when I switched my underlying database from sqlite to postgres and started getting errors about open transactions that had not existed with sqlite. Of course, I don't have tons of code to maintain that uses peewee that would be negatively affected by this change.

Regardless, I'd argue that - semantically speaking - autocommit's current behavior is broken.

[coleifer]

Added fbd6c20aa3fa75b1a92b74030f201d0e43304e28 which adds a new parameter to the Database class autorollback:

db = PostgresqlDatabase('my_db', autocommit=True, autorollback=True)

[coleifer]

Docs: http://peewee.readthedocs.org/en/latest/peewee/api.html#database-and-its-subclasses

[romanek-adam]

I was hit by this issue too. The default value for autorollback parameter is wrong, but I guess it's set this way to maintain backward compatibility?

Nevertheless I assumed a rollback would be issued in case of any exception, but apparently I was wrong...

[coleifer]

What do you mean it's wrong? It's documented as being False.

[romanek-adam]

Yes, it is. But as pointed out in previous comments most people rely on deafults and this is not the expected behavior in most cases.

[rudyryk]

@romanek-adam But it is documented and backward compatible! It's generally not a good idea to change defaults one day.

Probably it would be good to add some notes on autorollback to Transactions section in docs