override join_transaction_mode if engine_connect leaves the transaction open

[RazerM]

Describe the bug

I'm migrating our codebase to SQLAlchemy 2.0. After enabling future=True, I observed that sessions were being rolled back when session.commit() is called.

I've minimised the problem to an example that reproduces on SQLAlchemy 1.4 and 2.0.

Optional link from https://docs.sqlalchemy.org which documents the behavior that is expected

No response

SQLAlchemy Version in Use

1.4.52, 2.0.28

DBAPI (i.e. the database driver)

appears to be any but psycopg2 and whatever sqlite:// uses by default

Database Vendor and Major Version

PostgreSQL 12, 14

Python Version

3.10

Operating system

Linux, macOS

To Reproduce

from sqlalchemy import Column, Integer, MetaData, Table, create_engine, event, text
from sqlalchemy.orm import Session

metadata = MetaData()

foo = Table("foo", metadata, Column("id", Integer, primary_key=True))

engine = create_engine("sqlite:///:memory:", echo=True, future=True)

foo.create(bind=engine)


@event.listens_for(engine, "engine_connect")
def on_engine_connect(connection, branch):
    """If this event handler executes any SQL, the later session.commit()
    emits ROLLBACK instead of COMMIT.
    """
    connection.execute(text("SELECT 42"))


session = Session(bind=engine, future=True)
session.execute(foo.insert())
session.commit()  # Does not commit!

Error

The log output below shows a ROLLBACK instead of the expected COMMIT.

2024-03-15 15:43:27,688 INFO sqlalchemy.engine.Engine BEGIN (implicit)
2024-03-15 15:43:27,689 INFO sqlalchemy.engine.Engine 
CREATE TABLE foo (
	id INTEGER NOT NULL, 
	PRIMARY KEY (id)
)


2024-03-15 15:43:27,689 INFO sqlalchemy.engine.Engine [no key 0.00015s] ()
2024-03-15 15:43:27,690 INFO sqlalchemy.engine.Engine COMMIT
2024-03-15 15:43:27,691 INFO sqlalchemy.engine.Engine BEGIN (implicit)
2024-03-15 15:43:27,691 INFO sqlalchemy.engine.Engine SELECT 42
2024-03-15 15:43:27,691 INFO sqlalchemy.engine.Engine [generated in 0.00018s] ()
2024-03-15 15:43:27,692 INFO sqlalchemy.engine.Engine INSERT INTO foo DEFAULT VALUES
2024-03-15 15:43:27,692 INFO sqlalchemy.engine.Engine [generated in 0.00009s] ()
2024-03-15 15:43:27,692 INFO sqlalchemy.engine.Engine ROLLBACK

Additional context

I think I understand that the transaction changes in SQLAlchemy 2 is the cause. I was able to change our code to use a different event instead so I don't need a solution, but I feel like this interaction is extremely non-obvious and maybe the events need documentation about managing transactions which may affect the session.

[zzzeek]

the Session's handling of a connection is determined by the join_transaction_mode setting, which defaults to "conditional_savepoint". This means if the Session is handed a Connection that's already in a transaction, it assumes you are controlling that transaction externally.

So to have the Session not use that mode here, options are either make sure Connection is not in a transaction:

@event.listens_for(engine, "engine_connect")
def on_engine_connect(connection, branch):
    """If this event handler executes any SQL, the later session.commit()
    emits ROLLBACK instead of COMMIT.
    """
    connection.execute(text("SELECT 42"))
    connection.rollback()

or set join transaction to control_fully:

session = Session(bind=engine, future=True, join_transaction_mode="control_fully")
session.execute(foo.insert())
session.commit()

im not sure where would be the best place to add a note for this.

[RazerM]

I did try both of the snippets you show when sussing out it was the SQLAlchemy 2 transaction changes. It's worth noting that join_transaction_mode's docs say "when a given bind is a Connection" — the bind here is an engine.

[zzzeek]

we should adjust that doc to include "including when the connection is given to it by an Engine that has had its behavior modified by an event hook"

[zzzeek]

overall I think we need a note on "engine_connect" that starting a transaction inside the event changes the expected behavior of the engine if that transaction is left open

[sqla-tester]

Federico Caselli has proposed a fix for this issue in the main branch:

Improve docs wrt transactions and event with session https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5228

[sqla-tester]

Federico Caselli has proposed a fix for this issue in the rel_2_0 branch:

Improve docs wrt transactions and event with session https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5231