From: Mike Bayer Date: Sun, 2 May 2021 14:19:16 +0000 (-0400) Subject: restore legacy begin_nested()->root transaction behavior X-Git-Tag: rel_1_4_13~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ee7a82d71783bf71f3a95550624740e908d178a0;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git restore legacy begin_nested()->root transaction behavior Restored a legacy transactional behavior that was inadvertently removed from the :class:`_engine.Connection` as it was never tested as a known use case in previous versions, where calling upon the :meth:`_engine.Connection.begin_nested` method, when no transaction were present, would not create a SAVEPOINT at all, and would instead only start the outermost transaction alone, and return that :class:`.RootTransaction` object, acting like the outermost transaction. Committing the transaction object returned by :meth:`_engine.Connection.begin_nested` would therefore emit a real COMMIT on the database connection. This behavior is not at all what the 2.0 style connection will do - in 2.0 style, calling :meth:`_future.Connection.begin_nested` will "autobegin" the outer transaction, and then as instructed emit a SAVEPOINT, returning the :class:`.NestedTransaction` object. The outer transaction is committed by calling upon :meth:`_future.Connection.commit`, as is "commit-as-you-go" style usage. In non-"future" mode, while the old behavior is restored, it also emits a 2.0 deprecation warning as this is a legacy behavior. Additionally clarifies and reformats various engine-related documentation, in particular future connection.begin() which was a tire fire. Fixes: #6408 Change-Id: I4b81cc6b481b5493eef4c91bebc03210e2206d39 --- diff --git a/doc/build/changelog/unreleased_14/6408.rst b/doc/build/changelog/unreleased_14/6408.rst new file mode 100644 index 0000000000..70653fb3b2 --- /dev/null +++ b/doc/build/changelog/unreleased_14/6408.rst @@ -0,0 +1,24 @@ +.. change:: + :tags: bug, engine, regression + :tickets: 6408 + + Restored a legacy transactional behavior that was inadvertently removed + from the :class:`_engine.Connection` as it was never tested as a known use + case in previous versions, where calling upon the + :meth:`_engine.Connection.begin_nested` method, when no transaction were + present, would not create a SAVEPOINT at all, and would instead only start + the outermost transaction alone, and return that :class:`.RootTransaction` + object, acting like the outermost transaction. Committing the transaction + object returned by :meth:`_engine.Connection.begin_nested` would therefore + emit a real COMMIT on the database connection. + + This behavior is not at all what the 2.0 style connection will do - in 2.0 + style, calling :meth:`_future.Connection.begin_nested` will "autobegin" the + outer transaction, and then as instructed emit a SAVEPOINT, returning the + :class:`.NestedTransaction` object. The outer transaction is committed by + calling upon :meth:`_future.Connection.commit`, as is "commit-as-you-go" + style usage. + + In non-"future" mode, while the old behavior is restored, it also + emits a 2.0 deprecation warning as this is a legacy behavior. + diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index e0ebb4a989..293dc21b4e 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -694,10 +694,18 @@ class Connection(Connectable): which completes when either the :meth:`.Transaction.rollback` or :meth:`.Transaction.commit` method is called. - Nested calls to :meth:`.begin` on the same :class:`_engine.Connection` - will return new :class:`.Transaction` objects that represent - an emulated transaction within the scope of the enclosing - transaction, that is:: + .. tip:: + + The :meth:`_engine.Connection.begin` method is invoked when using + the :meth:`_engine.Engine.begin` context manager method as well. + All documentation that refers to behaviors specific to the + :meth:`_engine.Connection.begin` method also apply to use of the + :meth:`_engine.Engine.begin` method. + + Legacy use: nested calls to :meth:`.begin` on the same + :class:`_engine.Connection` will return new :class:`.Transaction` + objects that represent an emulated transaction within the scope of the + enclosing transaction, that is:: trans = conn.begin() # outermost transaction trans2 = conn.begin() # "nested" @@ -710,6 +718,14 @@ class Connection(Connectable): :class:`.Transaction` objects will roll back the transaction. + .. tip:: + + The above "nesting" behavior is a legacy behavior specific to + :term:`1.x style` use and will be removed in SQLAlchemy 2.0. For + notes on :term:`2.0 style` use, see + :meth:`_future.Connection.begin`. + + .. seealso:: :meth:`_engine.Connection.begin_nested` - use a SAVEPOINT @@ -745,9 +761,9 @@ class Connection(Connectable): return MarkerTransaction(self) def begin_nested(self): - """Begin a nested transaction and return a transaction handle. - - The returned object is an instance of :class:`.NestedTransaction`. + """Begin a nested transaction (i.e. SAVEPOINT) and return a + transaction handle, assuming an outer transaction is already + established. Nested transactions require SAVEPOINT support in the underlying database. Any transaction in the hierarchy may @@ -755,6 +771,36 @@ class Connection(Connectable): still controls the overall ``commit`` or ``rollback`` of the transaction of a whole. + The legacy form of :meth:`_engine.Connection.begin_nested` method has + alternate behaviors based on whether or not the + :meth:`_engine.Connection.begin` method was called previously. If + :meth:`_engine.Connection.begin` was not called, then this method will + behave the same as the :meth:`_engine.Connection.begin` method and + return a :class:`.RootTransaction` object that begins and commits a + real transaction - **no savepoint is invoked**. If + :meth:`_engine.Connection.begin` **has** been called, and a + :class:`.RootTransaction` is already established, then this method + returns an instance of :class:`.NestedTransaction` which will invoke + and manage the scope of a SAVEPOINT. + + .. tip:: + + The above mentioned behavior of + :meth:`_engine.Connection.begin_nested` is a legacy behavior + specific to :term:`1.x style` use. In :term:`2.0 style` use, the + :meth:`_future.Connection.begin_nested` method instead autobegins + the outer transaction that can be committed using + "commit-as-you-go" style; see + :meth:`_future.Connection.begin_nested` for migration details. + + .. versionchanged:: 1.4.13 The behavior of + :meth:`_engine.Connection.begin_nested` + as returning a :class:`.RootTransaction` if + :meth:`_engine.Connection.begin` were not called has been restored + as was the case in 1.3.x versions; in previous 1.4.x versions, an + outer transaction would be "autobegun" but would not be committed. + + .. seealso:: :meth:`_engine.Connection.begin` @@ -768,7 +814,18 @@ class Connection(Connectable): return self.__branch_from.begin_nested() if self._transaction is None: - self.begin() + if not self._is_future: + util.warn_deprecated_20( + "Calling Connection.begin_nested() in 2.0 style use will " + "return a NestedTransaction (SAVEPOINT) in all cases, " + "that will not commit the outer transaction. For code " + "that is cross-compatible between 1.x and 2.0 style use, " + "ensure Connection.begin() is called before calling " + "Connection.begin_nested()." + ) + return self.begin() + else: + self._autobegin() return NestedTransaction(self) @@ -2348,12 +2405,21 @@ class RootTransaction(Transaction): """Represent the "root" transaction on a :class:`_engine.Connection`. This corresponds to the current "BEGIN/COMMIT/ROLLBACK" that's occurring - for the :class:`_engine.Connection`. - - The :class:`_engine.RootTransaction` object is accessible via the - :attr:`_engine.Connection.get_transaction` method of + for the :class:`_engine.Connection`. The :class:`_engine.RootTransaction` + is created by calling upon the :meth:`_engine.Connection.begin` method, and + remains associated with the :class:`_engine.Connection` throughout its + active span. The current :class:`_engine.RootTransaction` in use is + accessible via the :attr:`_engine.Connection.get_transaction` method of :class:`_engine.Connection`. + In :term:`2.0 style` use, the :class:`_future.Connection` also employs + "autobegin" behavior that will create a new + :class:`_engine.RootTransaction` whenever a connection in a + non-transactional state is used to emit commands on the DBAPI connection. + The scope of the :class:`_engine.RootTransaction` in 2.0 style + use can be controlled using the :meth:`_future.Connection.commit` and + :meth:`_future.Connection.rollback` methods. + """ @@ -2899,14 +2965,14 @@ class Engine(Connectable, log.Identified): is committed. If an error is raised, the :class:`.Transaction` is rolled back. - The ``close_with_result`` flag is normally ``False``, and indicates - that the :class:`_engine.Connection` will be closed when the operation - is complete. When set to ``True``, it indicates the + Legacy use only: the ``close_with_result`` flag is normally ``False``, + and indicates that the :class:`_engine.Connection` will be closed when + the operation is complete. When set to ``True``, it indicates the :class:`_engine.Connection` is in "single use" mode, where the :class:`_engine.CursorResult` returned by the first call to :meth:`_engine.Connection.execute` will close the - :class:`_engine.Connection` when - that :class:`_engine.CursorResult` has exhausted all result rows. + :class:`_engine.Connection` when that :class:`_engine.CursorResult` has + exhausted all result rows. .. seealso:: diff --git a/lib/sqlalchemy/future/engine.py b/lib/sqlalchemy/future/engine.py index efd0b0eab9..cee17a432d 100644 --- a/lib/sqlalchemy/future/engine.py +++ b/lib/sqlalchemy/future/engine.py @@ -87,6 +87,11 @@ class Connection(_LegacyConnection): def begin(self): """Begin a transaction prior to autobegin occurring. + The returned object is an instance of :class:`_engine.RootTransaction`. + This object represents the "scope" of the transaction, + which completes when either the :meth:`_engine.Transaction.rollback` + or :meth:`_engine.Transaction.commit` method is called. + The :meth:`_future.Connection.begin` method in SQLAlchemy 2.0 begins a transaction that normally will be begun in any case when the connection is first used to execute a statement. The reason this method might be @@ -105,7 +110,8 @@ class Connection(_LegacyConnection): The above code is not fundamentally any different in its behavior than the following code which does not use - :meth:`_future.Connection.begin`:: + :meth:`_future.Connection.begin`; the below style is referred towards + as "commit as you go" style:: with engine.connect() as conn: conn.execute(...) @@ -116,53 +122,19 @@ class Connection(_LegacyConnection): conn.execute(...) conn.commit() - In both examples, if an exception is raised, the transaction will not - be committed. An explicit rollback of the transaction will occur, - including that the :meth:`_events.ConnectionEvents.rollback` event will - be emitted, as connection's context manager will call - :meth:`_future.Connection.close`, which will call - :meth:`_future.Connection.rollback` for any transaction in place - (excluding that of a SAVEPOINT). - From a database point of view, the :meth:`_future.Connection.begin` method does not emit any SQL or change the state of the underlying DBAPI connection in any way; the Python DBAPI does not have any concept of explicit transaction begin. - :return: a :class:`_engine.Transaction` object. This object supports - context-manager operation which will commit a transaction or - emit a rollback in case of error. - - . If this event is not being used, then there is - no real effect from invoking :meth:`_future.Connection.begin` ahead - of time as the Python DBAPI does not implement any explicit BEGIN - - - The returned object is an instance of :class:`_engine.Transaction`. - This object represents the "scope" of the transaction, - which completes when either the :meth:`_engine.Transaction.rollback` - or :meth:`_engine.Transaction.commit` method is called. - - Nested calls to :meth:`_future.Connection.begin` on the same - :class:`_future.Connection` will return new - :class:`_engine.Transaction` objects that represent an emulated - transaction within the scope of the enclosing transaction, that is:: - - trans = conn.begin() # outermost transaction - trans2 = conn.begin() # "nested" - trans2.commit() # does nothing - trans.commit() # actually commits - - Calls to :meth:`_engine.Transaction.commit` only have an effect when - invoked via the outermost :class:`_engine.Transaction` object, though - the :meth:`_engine.Transaction.rollback` method of any of the - :class:`_engine.Transaction` objects will roll back the transaction. - .. seealso:: + :ref:`tutorial_working_with_transactions` - in the + :ref:`unified_tutorial` + :meth:`_future.Connection.begin_nested` - use a SAVEPOINT - :meth:`_future.Connection.begin_twophase` - + :meth:`_engine.Connection.begin_twophase` - use a two phase /XID transaction :meth:`_future.Engine.begin` - context manager available from @@ -172,7 +144,8 @@ class Connection(_LegacyConnection): return super(Connection, self).begin() def begin_nested(self): - """Begin a nested transaction and return a transaction handle. + """Begin a nested transaction (i.e. SAVEPOINT) and return a transaction + handle. The returned object is an instance of :class:`_engine.NestedTransaction`. @@ -183,9 +156,21 @@ class Connection(_LegacyConnection): still controls the overall ``commit`` or ``rollback`` of the transaction of a whole. - In SQLAlchemy 2.0, the :class:`_engine.NestedTransaction` remains - independent of the :class:`_future.Connection` object itself. Calling - the :meth:`_future.Connection.commit` or + If an outer :class:`.RootTransaction` is not present on this + :class:`_future.Connection`, a new one is created using "autobegin". + This outer transaction may be completed using "commit-as-you-go" style + usage, by calling upon :meth:`_future.Connection.commit` or + :meth:`_future.Connection.rollback`. + + .. tip:: + + The "autobegin" behavior of :meth:`_future.Connection.begin_nested` + is specific to :term:`2.0 style` use; for legacy behaviors, see + :meth:`_engine.Connection.begin_nested`. + + The :class:`_engine.NestedTransaction` remains independent of the + :class:`_future.Connection` object itself. Calling the + :meth:`_future.Connection.commit` or :meth:`_future.Connection.rollback` will always affect the actual containing database transaction itself, and not the SAVEPOINT itself. When a database transaction is committed, any SAVEPOINTs that have been diff --git a/test/engine/test_transaction.py b/test/engine/test_transaction.py index 4751963e4f..d78ff7beeb 100644 --- a/test/engine/test_transaction.py +++ b/test/engine/test_transaction.py @@ -204,6 +204,37 @@ class TransactionTest(fixtures.TablesTest): ], ) + def test_ctxmanager_commits_real_trans_from_nested(self, local_connection): + m1 = mock.Mock() + + event.listen( + local_connection, "rollback_savepoint", m1.rollback_savepoint + ) + event.listen( + local_connection, "release_savepoint", m1.release_savepoint + ) + event.listen(local_connection, "rollback", m1.rollback) + event.listen(local_connection, "commit", m1.commit) + event.listen(local_connection, "begin", m1.begin) + event.listen(local_connection, "savepoint", m1.savepoint) + + with testing.expect_deprecated_20( + r"Calling Connection.begin_nested\(\) in 2.0 style use will return" + ): + with local_connection.begin_nested() as nested_trans: + pass + + assert not nested_trans.is_active + assert nested_trans._deactivated_from_connection + # legacy mode, no savepoint at all + eq_( + m1.mock_calls, + [ + mock.call.begin(local_connection), + mock.call.commit(local_connection), + ], + ) + def test_deactivated_warning_straight(self, local_connection): with expect_warnings( "transaction already deassociated from connection" @@ -1173,7 +1204,11 @@ class ResetAgentTest(ResetFixture, fixtures.TestBase): @testing.requires.savepoints def test_begin_nested_close(self, reset_agent): with reset_agent.engine.connect() as connection: - trans = connection.begin_nested() + with testing.expect_deprecated_20( + r"Calling Connection.begin_nested\(\) in " + r"2.0 style use will return" + ): + trans = connection.begin_nested() assert not trans.is_active eq_( reset_agent.mock_calls, @@ -1674,6 +1709,40 @@ class FutureTransactionTest(fixtures.FutureEngineMixin, fixtures.TablesTest): eq_(m1.mock_calls, [mock.call.rollback(local_connection)]) + @testing.requires.savepoints + def test_ctxmanager_autobegins_real_trans_from_nested( + self, local_connection + ): + m1 = mock.Mock() + + event.listen( + local_connection, "rollback_savepoint", m1.rollback_savepoint + ) + event.listen( + local_connection, "release_savepoint", m1.release_savepoint + ) + event.listen(local_connection, "rollback", m1.rollback) + event.listen(local_connection, "commit", m1.commit) + event.listen(local_connection, "begin", m1.begin) + event.listen(local_connection, "savepoint", m1.savepoint) + + with local_connection.begin_nested() as nested_trans: + pass + + assert not nested_trans.is_active + assert nested_trans._deactivated_from_connection + # legacy mode, no savepoint at all + eq_( + m1.mock_calls, + [ + mock.call.begin(local_connection), + mock.call.savepoint(local_connection, mock.ANY), + mock.call.release_savepoint( + local_connection, mock.ANY, mock.ANY + ), + ], + ) + def test_explicit_begin(self): users = self.tables.users