From: Federico Caselli Date: Fri, 15 Mar 2024 18:15:27 +0000 (+0100) Subject: Ignore join_transaction_mode when bind is an engine X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=95054db1bff0b409fa36da9228008c4de95c970b;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Ignore join_transaction_mode when bind is an engine Ignore :paramref:`_orm.Session.join_transaction_mode` in all cases when the bind provided to the :class:`_orm.Session` is an class:`_engine.Engine`. Previously if an event that executed before the session logic, like :meth:`_engine.ConnectionEvents.engine_connect`, left the connection with an active transaction, the paramref:`_orm.Session.join_transaction_mode` behavior took place, leading to a surprising behavior. Fixes: #11163 Change-Id: I10147876d07352f2dab898d615e98a9acd6eb91b --- diff --git a/doc/build/changelog/unreleased_20/11163.rst b/doc/build/changelog/unreleased_20/11163.rst new file mode 100644 index 0000000000..da21b45378 --- /dev/null +++ b/doc/build/changelog/unreleased_20/11163.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: orm + :tickets: 11163 + + Added a warning noting when an + :meth:`_engine.ConnectionEvents.engine_connect` event may be leaving + a transaction open, which can alter the behavior of a + :class:`_orm.Session` using such an engine as bind. + On SQLAlchemy 2.1 :paramref:`_orm.Session.join_transaction_mode` will + instead be ignored in all cases when the session bind is + an :class:`_engine.Engine`. diff --git a/doc/build/changelog/unreleased_21/11163.rst b/doc/build/changelog/unreleased_21/11163.rst new file mode 100644 index 0000000000..c835571458 --- /dev/null +++ b/doc/build/changelog/unreleased_21/11163.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: orm + :tickets: 11163 + + Ignore :paramref:`_orm.Session.join_transaction_mode` in all cases when + the bind provided to the :class:`_orm.Session` is an + :class:`_engine.Engine`. + Previously if an event that executed before the session logic, + like :meth:`_engine.ConnectionEvents.engine_connect`, + left the connection with an active transaction, the + :paramref:`_orm.Session.join_transaction_mode` behavior took + place, leading to a surprising behavior. diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index c495a964f3..a23239e098 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -1208,30 +1208,38 @@ class SessionTransaction(_StateChange, TransactionalContext): elif self.nested: transaction = conn.begin_nested() elif conn.in_transaction(): - join_transaction_mode = self.session.join_transaction_mode - if join_transaction_mode == "conditional_savepoint": - if conn.in_nested_transaction(): - join_transaction_mode = "create_savepoint" - else: - join_transaction_mode = "rollback_only" - - if join_transaction_mode in ( - "control_fully", - "rollback_only", - ): - if conn.in_nested_transaction(): - transaction = ( - conn._get_required_nested_transaction() - ) - else: - transaction = conn._get_required_transaction() - if join_transaction_mode == "rollback_only": - should_commit = False - elif join_transaction_mode == "create_savepoint": - transaction = conn.begin_nested() + if local_connect: + _trans = conn.get_transaction() + assert _trans is not None + transaction = _trans else: - assert False, join_transaction_mode + join_transaction_mode = ( + self.session.join_transaction_mode + ) + + if join_transaction_mode == "conditional_savepoint": + if conn.in_nested_transaction(): + join_transaction_mode = "create_savepoint" + else: + join_transaction_mode = "rollback_only" + + if join_transaction_mode in ( + "control_fully", + "rollback_only", + ): + if conn.in_nested_transaction(): + transaction = ( + conn._get_required_nested_transaction() + ) + else: + transaction = conn._get_required_transaction() + if join_transaction_mode == "rollback_only": + should_commit = False + elif join_transaction_mode == "create_savepoint": + transaction = conn.begin_nested() + else: + assert False, join_transaction_mode else: transaction = conn.begin() except: diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py index e502a88833..eda7811846 100644 --- a/test/orm/test_transaction.py +++ b/test/orm/test_transaction.py @@ -108,7 +108,7 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): trans.commit() assert len(sess.query(User).all()) == 1 - @testing.variation( + join_transaction_mode = testing.variation( "join_transaction_mode", [ "none", @@ -118,6 +118,8 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): "rollback_only", ], ) + + @join_transaction_mode @testing.variation("operation", ["commit", "close", "rollback", "nothing"]) @testing.variation("external_state", ["none", "transaction", "savepoint"]) def test_join_transaction_modes( @@ -243,6 +245,57 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): else: external_state.fail() + @join_transaction_mode + @testing.variation("operation", ["commit", "close", "rollback"]) + def test_join_transaction_mode_with_event( + self, join_transaction_mode, operation + ): + eng = engines.testing_engine() + eng_conn = None + events = [] + + @event.listens_for(eng, "commit") + def on_commit(conn): + events.append("commit") + + @event.listens_for(eng, "rollback") + def on_rollback(conn): + events.append("rollback") + + @event.listens_for(eng.pool, "checkin") + def on_checkin(conn, record): + events.append("checkin") + + @event.listens_for(eng, "engine_connect") + def make_stat(conn): + nonlocal eng_conn + eng_conn = conn + conn.begin() + + if join_transaction_mode.none: + s = Session(eng) + else: + s = Session(eng, join_transaction_mode=join_transaction_mode.name) + + s.connection() + + expected = [] + if operation.commit: + s.commit() + expected.append("commit") + elif operation.rollback: + s.rollback() + expected.append("rollback") + elif operation.close: + s.close() + expected.append("rollback") + else: + operation.fail() + is_(eng_conn.in_transaction(), False) + + expected.append("checkin") + eq_(events, expected) + def test_subtransaction_on_external_commit(self, connection_no_trans): users, User = self.tables.users, self.classes.User @@ -839,7 +892,10 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): return_value=mock.Mock( _is_future=False, execution_options=mock.Mock( - return_value=mock.Mock(_is_future=False) + return_value=mock.Mock( + _is_future=False, + in_transaction=mock.Mock(return_value=False), + ) ), ) ) @@ -857,7 +913,9 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): def test_execution_options_ignored_mid_transaction(self): bind = mock.Mock() - conn = mock.Mock(engine=bind) + conn = mock.Mock( + engine=bind, in_transaction=mock.Mock(return_value=False) + ) bind.connect = mock.Mock(return_value=conn) sess = Session(bind=bind) sess.execute(text("select 1"))