don't allow implicit inactive transactions on connection

[Wim-De-Clercq]

Not sure if question or bug...

I'm using https://docs.sqlalchemy.org/en/13/orm/session_transaction.html#joining-a-session-into-an-external-transaction-such-as-for-test-suites as a guide.

This is the code

import unittest

from sqlalchemy import Column
from sqlalchemy import Integer
from sqlalchemy import String
from sqlalchemy import create_engine
from sqlalchemy import event
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import Session

Base = declarative_base()


class User(Base):
    __tablename__ = "users"

    id = Column('id', Integer(), primary_key=True, nullable=False)
    name = Column('name', String(128), nullable=False)


class SomeTest(unittest.TestCase):
    def setUp(self):
        # connect to the database
        engine = create_engine('postgresql://u:p@localhost:5432/test_db', echo=True)
        Base.metadata.bind = engine
        Base.metadata.create_all(engine)
        self.connection = engine.connect()

        # begin a non-ORM transaction
        self.trans = self.connection.begin()

        # bind an individual Session to the connection
        self.session = Session(bind=self.connection)

        # then each time that SAVEPOINT ends, reopen it
        @event.listens_for(self.session, "after_transaction_end")
        def restart_savepoint(session, transaction):
            if transaction.nested and not transaction._parent.nested:

                # ensure that state is expired the way
                # session.commit() at the top level normally does
                # (optional step)
                session.expire_all()

                session.begin_nested()

    def test_1(self):
        # use the session in tests.
        user = User(name='name')
        self.session.add(user)
        # Without flush it works, even a commit works, flush alone is bad
        # Doing a session.query(...).all() is also bad
        self.session.flush()  #

        self.session.rollback()
        self.session.add(user)
        self.session.commit()

    def test_2(self):
        # use the session in tests.
        result = self.session.query(User).all()
        print("test_2:", [res.name for res in result])

    def tearDown(self):
        self.session.close()

        # rollback - everything that happened with the
        # Session above (including calls to commit())
        # is rolled back.
        self.trans.rollback()

        # return connection to the Engine
        self.connection.close()

It's important that test 1 runs before test 2 on your system.
You can also just run test 1 alone, and see users being added to your database, even though it is meant to rollback.

I am expecting test 2 to not see the user that is added by test 1.
But that session.flush() for some reason makes the whole test transaction rollback fail and the user is actually persisted in the database.

libraries

psycopg2   2.8.2  
SQLAlchemy 1.3.4 

database: psql (10.5 (Debian 10.5-1.pgdg90+1))

How should I setup a test like this?
It's not that I want to flush, but I actually want to do a session.query(User).all(), but that has the same effect: User gets persisted.
I assume the reason is that it never enters the if-statement in the listener, but then the documentation is wrong?

[Wim-De-Clercq]

Of course just as i type this all out, I realise I forgot begin_nested().
Nevermind the issue then.

[zzzeek]

heh, you know what, while I've likely seen this kind of thing many times over the years, today I think it is a bug. The transaction has been rolled back in your test, but your outer transaction has not been rolled back, then the connection keeps emitting SQL on a new transaction. The outermost transaction you've created is now a do-nothing object. objects that go into a "do nothing" state implicitly create confusion like this. I had to stare for awhile at the code to see where it went wrong.

The confusion of this is kind of validating to the behavior that the Session has in this regard, which is is that if you roll back an "inner" "sub" transaction, it won't let anything happen until you rollback all the way out to the outermost transaction block. This is the behavior I struggled to defend in https://docs.sqlalchemy.org/en/13/faq/sessions.html#but-why-isn-t-the-one-automatic-call-to-rollback-enough-why-must-i-rollback-again . In this case, it would have made this situation much easier to spot.

I have a patch coming for 1.4 only, since this is quite a fundamental behavioral change, where your test above will emit:

sqlalchemy.exc.InvalidRequestError: This connection is on an inactive transaction. Please rollback() fully before proceeding. (Background on this error at: http://sqlalche.me/e/8s2a)

[Wim-De-Clercq]

@zzzeek Thanks for your explanation, (and for even looking at closed bugs)
Before I make another question/issue for what i just found, is this a similar thing then because I got 2 transactions (I assume there's 1 implicit transaction started for the first query) and then I rollback another one?

I can't really grasp why the commit makes a difference.

from sqlalchemy import Column
from sqlalchemy import Integer
from sqlalchemy import String
from sqlalchemy import create_engine
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker

Base = declarative_base()


class User(Base):
    __tablename__ = "users"

    id = Column('id', Integer(), primary_key=True, nullable=False)
    name = Column('name', String(128), nullable=False)

engine = create_engine('postgresql://u:p@localhost:5432/test_db', echo=True)
Base.metadata.bind = engine
Base.metadata.create_all(engine)
connection = engine.connect()
Session = sessionmaker(bind=connection)

# End of basic setup

session = Session()
session.query(User).all() # this is crucial
# with the below commit, this code runs fine. Without it crashes.
# session.commit()

transaction = connection.begin()
transaction.rollback()

session.commit() # Error: This transaction is inactive

  File "/media/wim/DATA/projects/playing/.venv2/lib/python2.7/site-packages/sqlalchemy/orm/session.py", line 1026, in commit
    self.transaction.commit()
  File "/media/wim/DATA/projects/playing/.venv2/lib/python2.7/site-packages/sqlalchemy/orm/session.py", line 497, in commit
    t[1].commit()
  File "/media/wim/DATA/projects/playing/.venv2/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1715, in commit
    raise exc.InvalidRequestError("This transaction is inactive")

[zzzeek]

the Session is starting a "transaction" as soon as you ask it to do anything, like query.all(), and not before. it does this by calling "conn = bind.connect(); conn.begin()". When you bind the Session to a Connection and not an Engine, the "connect()" method just returns a Connection object that is a proxy for the Connection that's already there. So it calls begin(), makes a Transaction at the root. Then you make a subtransaction on that with your own begin(), then you roll it back. This makes the outer transaction inactive. The Session is still holding onto the actual Transaction object, so when it tries to commit, it fails. The change I'm making is that the Connection itself doesn't throw away these Transaction objects either until there is an explicit call to rollback().

[sqla-tester]

Mike Bayer has proposed a fix for this issue in the master branch:

Don't discard inactive transaction until it is explicitly rolled back https://gerrit.sqlalchemy.org/1302