Issue: PostgresqlDatabase.last_insert_id w/ autocommit=True leaves open transactions
Author: justinrosenthal
State: closed
Created: 2014-05-13 23:58:14

If you do a `Model.create` with a PostgresqlDatabase that has autocommit enabled, you'll be left with an unclosed transaction.  Since it's in autocommit mode, it's most likely the case that you won't close it yourself.

`InsertQuery.execute` returns `self.database.last_insert_id(self._execute(), self.model_class)`.  The `last_insert_id` method does a `cursor.execute('SELECT ...')` and then returns the result.  The `execute()` will have implicitly started a new transaction that doesn't get closed.  I think the solution might be as simple as adding a couple `self.commit()` calls, as shown below, but I don't know the codebase well enough to know if that's the right thing to do or not.

``` python
    def last_insert_id(self, cursor, model):
        meta = model._meta
        seq = meta.primary_key.sequence
        schema = ''
        if meta.schema:
            schema = '%s.' % meta.schema
        if seq:
            cursor.execute("SELECT CURRVAL('%s\"%s\"')" % (schema, seq))
            self.commit()   # <----------- HERE
            return cursor.fetchone()[0]
        elif meta.auto_increment:
            cursor.execute("SELECT CURRVAL('%s\"%s_%s_seq\"')" % (
                schema, meta.db_table, meta.primary_key.db_column))
            self.commit()    # <----------- AND HERE
            return cursor.fetchone()[0]
```


Are you sure about this? Any way you can demonstrate this behavior?
On May 13, 2014 6:58 PM, "Justin Rosenthal" notifications@github.com
wrote:

> If you do a Model.create with a PostgresqlDatabase that has autocommit
> enabled, you'll be left with an unclosed transaction. Since it's in
> autocommit mode, it's most likely the case that you won't close it yourself.
> 
> InsertQuery.execute returns self.database.last_insert_id(self._execute(),
> self.model_class). The last_insert_id method does a cursor.execute('SELECT
> ...') and then returns the result. The execute() will have implicitly
> started a new transaction that doesn't get closed. I think the solution
> might be as simple as adding a couple self.commit() calls, as shown
> below, but I don't know the codebase well enough to know if that's the
> right thing to do or not.
> 
> ```
> def last_insert_id(self, cursor, model):
>     meta = model._meta
>     seq = meta.primary_key.sequence
>     schema = ''
>     if meta.schema:
>         schema = '%s.' % meta.schema
>     if seq:
>         cursor.execute("SELECT CURRVAL('%s\"%s\"')" % (schema, seq))
>         self.commit()   # <----------- HERE
>         return cursor.fetchone()[0]
>     elif meta.auto_increment:
>         cursor.execute("SELECT CURRVAL('%s\"%s_%s_seq\"')" % (
>             schema, meta.db_table, meta.primary_key.db_column))
>         self.commit()    # <----------- AND HERE
>         return cursor.fetchone()[0]
> ```
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/coleifer/peewee/issues/353
> .


This is the best way I could think to prove the point:
https://gist.github.com/justinrosenthal/873ca0bfbf938ccf5689

You'll want to run that against a quiet database (optimally no other connections, so the SELECT only returns you a single row).

Note that the Event table does not have a primary key defined, which causes Postgres to assign one via a sequence.  If you specify your own primary key, `last_insert_id` doesn't run a SELECT, so you won't see the issue.


Yipes you're totally right.