From: Mike Bayer Date: Tue, 5 Aug 2025 14:46:21 +0000 (-0400) Subject: implement skip_autocommit_rollback X-Git-Tag: rel_2_0_43~5^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=eea260f5bed9f64a8ae9d993b4972b754524078a;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git implement skip_autocommit_rollback Added new parameter :paramref:`.create_engine.skip_autocommit_rollback` which provides for a per-dialect feature of preventing the DBAPI ``.rollback()`` from being called under any circumstances, if the connection is detected as being in "autocommit" mode. This improves upon a critical performance issue identified in MySQL dialects where the network overhead of the ``.rollback()`` call remains prohibitive even if autocommit mode is set. Fixes: #12784 Change-Id: I22b45ab2fc396c5aadeff5cdc5ce895144d00098 (cherry picked from commit 99da878a25fd5105f17893f16617e7413e32a3e8) --- diff --git a/doc/build/changelog/unreleased_20/12784.rst b/doc/build/changelog/unreleased_20/12784.rst new file mode 100644 index 0000000000..ee1eeb0319 --- /dev/null +++ b/doc/build/changelog/unreleased_20/12784.rst @@ -0,0 +1,15 @@ +.. change:: + :tags: usecase, engine + :tickets: 12784 + + Added new parameter :paramref:`.create_engine.skip_autocommit_rollback` + which provides for a per-dialect feature of preventing the DBAPI + ``.rollback()`` from being called under any circumstances, if the + connection is detected as being in "autocommit" mode. This improves upon + a critical performance issue identified in MySQL dialects where the network + overhead of the ``.rollback()`` call remains prohibitive even if autocommit + mode is set. + + .. seealso:: + + :ref:`dbapi_autocommit_skip_rollback` diff --git a/doc/build/core/connections.rst b/doc/build/core/connections.rst index 030d41cd3b..e1c25474e5 100644 --- a/doc/build/core/connections.rst +++ b/doc/build/core/connections.rst @@ -285,7 +285,7 @@ that loses not only "read committed" but also loses atomicity. :ref:`dbapi_autocommit_understanding`, that "autocommit" isolation level like any other isolation level does **not** affect the "transactional" behavior of the :class:`_engine.Connection` object, which continues to call upon DBAPI - ``.commit()`` and ``.rollback()`` methods (they just have no effect under + ``.commit()`` and ``.rollback()`` methods (they just have no net effect under autocommit), and for which the ``.begin()`` method assumes the DBAPI will start a transaction implicitly (which means that SQLAlchemy's "begin" **does not change autocommit mode**). @@ -340,6 +340,8 @@ begin a transaction:: set at this level. This because the option must be set on a DBAPI connection on a per-transaction basis. +.. _dbapi_autocommit_engine: + Setting Isolation Level or DBAPI Autocommit for an Engine ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -358,14 +360,20 @@ With the above setting, each new DBAPI connection the moment it's created will be set to use a ``"REPEATABLE READ"`` isolation level setting for all subsequent operations. +.. tip:: + + Prefer to set frequently used isolation levels engine wide as illustrated + above compared to using per-engine or per-connection execution options for + maximum performance. + .. _dbapi_autocommit_multiple: Maintaining Multiple Isolation Levels for a Single Engine ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The isolation level may also be set per engine, with a potentially greater -level of flexibility, using either the -:paramref:`_sa.create_engine.execution_options` parameter to +level of flexibility but with a small per-connection performance overhead, +using either the :paramref:`_sa.create_engine.execution_options` parameter to :func:`_sa.create_engine` or the :meth:`_engine.Engine.execution_options` method, the latter of which will create a copy of the :class:`.Engine` that shares the dialect and connection pool of the original engine, but has its own @@ -408,6 +416,14 @@ copy of the original :class:`_engine.Engine`. Both ``eng`` and The isolation level setting, regardless of which one it is, is unconditionally reverted when a connection is returned to the connection pool. +.. note:: + + The execution options approach, whether used engine wide or per connection, + incurs a small performance penalty as isolation level instructions + are sent on connection acquire as well as connection release. Consider + the engine-wide isolation setting at :ref:`dbapi_autocommit_engine` so + that connections are configured at the target isolation level permanently + as they are pooled. .. seealso:: @@ -457,8 +473,9 @@ committed, this rollback has no change on the state of the database. It is important to note that "autocommit" mode persists even when the :meth:`_engine.Connection.begin` method is called; -the DBAPI will not emit any BEGIN to the database, nor will it emit -COMMIT when :meth:`_engine.Connection.commit` is called. This usage is also +the DBAPI will not emit any BEGIN to the database. When +:meth:`_engine.Connection.commit` is called, the DBAPI may still emit the +"COMMIT" instruction, but this is a no-op at the database level. This usage is also not an error scenario, as it is expected that the "autocommit" isolation level may be applied to code that otherwise was written assuming a transactional context; the "isolation level" is, after all, a configurational detail of the transaction @@ -483,7 +500,7 @@ it probably will have no effect due to autocommit mode: INFO sqlalchemy.engine.Engine BEGIN (implicit) ... - INFO sqlalchemy.engine.Engine COMMIT using DBAPI connection.commit(), DBAPI should ignore due to autocommit mode + INFO sqlalchemy.engine.Engine COMMIT using DBAPI connection.commit(), has no effect due to autocommit mode At the same time, even though we are using "DBAPI autocommit", SQLAlchemy's transactional semantics, that is, the in-Python behavior of :meth:`_engine.Connection.begin` @@ -514,6 +531,43 @@ maintain a completely consistent usage pattern with the :class:`_engine.Connection` where DBAPI-autocommit mode can be changed independently without indicating any code changes elsewhere. +.. _dbapi_autocommit_skip_rollback: + +Fully preventing ROLLBACK calls under autocommit +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +.. versionadded:: 2.0.43 + +A common use case is to use AUTOCOMMIT isolation mode to improve performance, +and this is a particularly common practice on MySQL / MariaDB databases. +When seeking this pattern, it should be preferred to set AUTOCOMMIT engine +wide using the :paramref:`.create_engine.isolation_level` so that pooled +connections are permanently set in autocommit mode. The SQLAlchemy connection +pool as well as the :class:`.Connection` will still seek to invoke the DBAPI +``.rollback()`` method upon connection :term:`reset`, as their behavior +remains agonstic of the isolation level that's configured on the connection. +As this rollback still incurs a network round trip under most if not all +DBAPI drivers, this additional network trip may be disabled using the +:paramref:`.create_engine.skip_autocommit_rollback` parameter, which will +apply a rule at the basemost portion of the dialect that invokes DBAPI +``.rollback()`` to first check if the connection is configured in autocommit, +using a method of detection that does not itself incur network overhead:: + + autocommit_engine = create_engine( + "mysql+mysqldb://scott:tiger@mysql80/test", + skip_autocommit_rollback=True, + isolation_level="AUTOCOMMIT", + ) + +When DBAPI connections are returned to the pool by the :class:`.Connection`, +whether the :class:`.Connection` or the pool attempts to reset the +"transaction", the underlying DBAPI ``.rollback()`` method will be blocked +based on a positive test of "autocommit". + +If the dialect in use does not support a no-network means of detecting +autocommit, the dialect will raise ``NotImplementedError`` when a connection +release is attempted. + Changing Between Isolation Levels ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/doc/build/core/pooling.rst b/doc/build/core/pooling.rst index 1a4865ba2b..8e21a74f33 100644 --- a/doc/build/core/pooling.rst +++ b/doc/build/core/pooling.rst @@ -134,8 +134,14 @@ The pool includes "reset on return" behavior which will call the ``rollback()`` method of the DBAPI connection when the connection is returned to the pool. This is so that any existing transactional state is removed from the connection, which includes not just uncommitted data but table and row locks as -well. For most DBAPIs, the call to ``rollback()`` is inexpensive, and if the -DBAPI has already completed a transaction, the method should be a no-op. +well. For most DBAPIs, the call to ``rollback()`` is relatively inexpensive. + +The "reset on return" feature takes place when a connection is :term:`released` +back to the connection pool. In modern SQLAlchemy, this reset on return +behavior is shared between the :class:`.Connection` and the :class:`.Pool`, +where the :class:`.Connection` itself, if it releases its transaction upon close, +considers ``.rollback()`` to have been called, and instructs the pool to skip +this step. Disabling Reset on Return for non-transactional connections @@ -146,24 +152,39 @@ using a connection that is configured for :ref:`autocommit ` or when using a database that has no ACID capabilities such as the MyISAM engine of MySQL, the reset-on-return behavior can be disabled, which is typically done for -performance reasons. This can be affected by using the +performance reasons. + +As of SQLAlchemy 2.0.43, the :paramref:`.create_engine.skip_autocommit_rollback` +parameter of :func:`.create_engine` provides the most complete means of +preventing ROLLBACK from being emitted while under autocommit mode, as it +blocks the DBAPI ``.rollback()`` method from being called by the dialect +completely:: + + autocommit_engine = create_engine( + "mysql+mysqldb://scott:tiger@mysql80/test", + skip_autocommit_rollback=True, + isolation_level="AUTOCOMMIT", + ) + +Detail on this pattern is at :ref:`dbapi_autocommit_skip_rollback`. + +The :class:`_pool.Pool` itself also has a parameter that can control its +"reset on return" behavior, noting that in modern SQLAlchemy this is not +the only path by which the DBAPI transaction is released, which is the :paramref:`_pool.Pool.reset_on_return` parameter of :class:`_pool.Pool`, which is also available from :func:`_sa.create_engine` as :paramref:`_sa.create_engine.pool_reset_on_return`, passing a value of ``None``. -This is illustrated in the example below, in conjunction with the -:paramref:`.create_engine.isolation_level` parameter setting of -``AUTOCOMMIT``:: +This pattern looks as below:: - non_acid_engine = create_engine( - "mysql://scott:tiger@host/db", + autocommit_engine = create_engine( + "mysql+mysqldb://scott:tiger@mysql80/test", pool_reset_on_return=None, isolation_level="AUTOCOMMIT", ) -The above engine won't actually perform ROLLBACK when connections are returned -to the pool; since AUTOCOMMIT is enabled, the driver will also not perform -any BEGIN operation. - +The above pattern will still see ROLLBACKs occur however as the :class:`.Connection` +object implicitly starts transaction blocks in the SQLAlchemy 2.0 series, +which still emit ROLLBACK independently of the pool's reset sequence. Custom Reset-on-Return Schemes ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/lib/sqlalchemy/connectors/pyodbc.py b/lib/sqlalchemy/connectors/pyodbc.py index 766493e2e0..dee26169ba 100644 --- a/lib/sqlalchemy/connectors/pyodbc.py +++ b/lib/sqlalchemy/connectors/pyodbc.py @@ -243,3 +243,8 @@ class PyODBCConnector(Connector): else: dbapi_connection.autocommit = False super().set_isolation_level(dbapi_connection, level) + + def detect_autocommit_setting( + self, dbapi_conn: interfaces.DBAPIConnection + ) -> bool: + return bool(dbapi_conn.autocommit) diff --git a/lib/sqlalchemy/dialects/mysql/aiomysql.py b/lib/sqlalchemy/dialects/mysql/aiomysql.py index e2ac70b029..af1ac2f334 100644 --- a/lib/sqlalchemy/dialects/mysql/aiomysql.py +++ b/lib/sqlalchemy/dialects/mysql/aiomysql.py @@ -98,6 +98,9 @@ class AsyncAdapt_aiomysql_connection(AsyncAdapt_dbapi_connection): def autocommit(self, value: Any) -> None: self.await_(self._connection.autocommit(value)) + def get_autocommit(self) -> bool: + return self._connection.get_autocommit() # type: ignore + def terminate(self) -> None: # it's not awaitable. self._connection.close() diff --git a/lib/sqlalchemy/dialects/mysql/asyncmy.py b/lib/sqlalchemy/dialects/mysql/asyncmy.py index 750735e8f1..61157facd3 100644 --- a/lib/sqlalchemy/dialects/mysql/asyncmy.py +++ b/lib/sqlalchemy/dialects/mysql/asyncmy.py @@ -104,6 +104,9 @@ class AsyncAdapt_asyncmy_connection(AsyncAdapt_dbapi_connection): def autocommit(self, value: Any) -> None: self.await_(self._connection.autocommit(value)) + def get_autocommit(self) -> bool: + return self._connection.get_autocommit() # type: ignore + def terminate(self) -> None: # it's not awaitable. self._connection.close() diff --git a/lib/sqlalchemy/dialects/mysql/mariadbconnector.py b/lib/sqlalchemy/dialects/mysql/mariadbconnector.py index c6bb58a8d9..b2d3d63a90 100644 --- a/lib/sqlalchemy/dialects/mysql/mariadbconnector.py +++ b/lib/sqlalchemy/dialects/mysql/mariadbconnector.py @@ -253,6 +253,9 @@ class MySQLDialect_mariadbconnector(MySQLDialect): "AUTOCOMMIT", ) + def detect_autocommit_setting(self, dbapi_conn: DBAPIConnection) -> bool: + return bool(dbapi_conn.autocommit) + def set_isolation_level( self, dbapi_connection: DBAPIConnection, level: IsolationLevel ) -> None: diff --git a/lib/sqlalchemy/dialects/mysql/mysqlconnector.py b/lib/sqlalchemy/dialects/mysql/mysqlconnector.py index a830cb5afe..feaf52084b 100644 --- a/lib/sqlalchemy/dialects/mysql/mysqlconnector.py +++ b/lib/sqlalchemy/dialects/mysql/mysqlconnector.py @@ -277,6 +277,9 @@ class MySQLDialect_mysqlconnector(MySQLDialect): "AUTOCOMMIT", ) + def detect_autocommit_setting(self, dbapi_conn: DBAPIConnection) -> bool: + return bool(dbapi_conn.autocommit) + def set_isolation_level( self, dbapi_connection: DBAPIConnection, level: IsolationLevel ) -> None: diff --git a/lib/sqlalchemy/dialects/mysql/mysqldb.py b/lib/sqlalchemy/dialects/mysql/mysqldb.py index de4ae61c04..a5b0ca203c 100644 --- a/lib/sqlalchemy/dialects/mysql/mysqldb.py +++ b/lib/sqlalchemy/dialects/mysql/mysqldb.py @@ -298,6 +298,9 @@ class MySQLDialect_mysqldb(MySQLDialect): "AUTOCOMMIT", ) + def detect_autocommit_setting(self, dbapi_conn: DBAPIConnection) -> bool: + return dbapi_conn.get_autocommit() # type: ignore[no-any-return] + def set_isolation_level( self, dbapi_connection: DBAPIConnection, level: IsolationLevel ) -> None: diff --git a/lib/sqlalchemy/dialects/oracle/cx_oracle.py b/lib/sqlalchemy/dialects/oracle/cx_oracle.py index 0514ebbcd4..69bb7f3e74 100644 --- a/lib/sqlalchemy/dialects/oracle/cx_oracle.py +++ b/lib/sqlalchemy/dialects/oracle/cx_oracle.py @@ -1234,6 +1234,9 @@ class OracleDialect_cx_oracle(OracleDialect): with dbapi_connection.cursor() as cursor: cursor.execute(f"ALTER SESSION SET ISOLATION_LEVEL={level}") + def detect_autocommit_setting(self, dbapi_conn) -> bool: + return bool(dbapi_conn.autocommit) + def _detect_decimal_char(self, connection): # we have the option to change this setting upon connect, # or just look at what it is upon connect and convert. diff --git a/lib/sqlalchemy/dialects/postgresql/_psycopg_common.py b/lib/sqlalchemy/dialects/postgresql/_psycopg_common.py index 9b09868bd3..0ff301e052 100644 --- a/lib/sqlalchemy/dialects/postgresql/_psycopg_common.py +++ b/lib/sqlalchemy/dialects/postgresql/_psycopg_common.py @@ -170,6 +170,9 @@ class _PGDialect_common_psycopg(PGDialect): def _do_autocommit(self, connection, value): connection.autocommit = value + def detect_autocommit_setting(self, dbapi_connection): + return bool(dbapi_connection.autocommit) + def do_ping(self, dbapi_connection): before_autocommit = dbapi_connection.autocommit diff --git a/lib/sqlalchemy/dialects/postgresql/asyncpg.py b/lib/sqlalchemy/dialects/postgresql/asyncpg.py index 36566b6740..5b3073af35 100644 --- a/lib/sqlalchemy/dialects/postgresql/asyncpg.py +++ b/lib/sqlalchemy/dialects/postgresql/asyncpg.py @@ -1117,6 +1117,9 @@ class PGDialect_asyncpg(PGDialect): def set_isolation_level(self, dbapi_connection, level): dbapi_connection.set_isolation_level(self._isolation_lookup[level]) + def detect_autocommit_setting(self, dbapi_conn) -> bool: + return bool(dbapi_conn.autocommit) + def set_readonly(self, connection, value): connection.readonly = value diff --git a/lib/sqlalchemy/dialects/postgresql/pg8000.py b/lib/sqlalchemy/dialects/postgresql/pg8000.py index bf113230e0..47016b4a35 100644 --- a/lib/sqlalchemy/dialects/postgresql/pg8000.py +++ b/lib/sqlalchemy/dialects/postgresql/pg8000.py @@ -540,6 +540,9 @@ class PGDialect_pg8000(PGDialect): cursor.execute("COMMIT") cursor.close() + def detect_autocommit_setting(self, dbapi_conn) -> bool: + return bool(dbapi_conn.autocommit) + def set_readonly(self, connection, value): cursor = connection.cursor() try: diff --git a/lib/sqlalchemy/dialects/sqlite/pysqlite.py b/lib/sqlalchemy/dialects/sqlite/pysqlite.py index 2f23886b54..1f9a55c549 100644 --- a/lib/sqlalchemy/dialects/sqlite/pysqlite.py +++ b/lib/sqlalchemy/dialects/sqlite/pysqlite.py @@ -502,6 +502,9 @@ class SQLiteDialect_pysqlite(SQLiteDialect): dbapi_connection.isolation_level = "" return super().set_isolation_level(dbapi_connection, level) + def detect_autocommit_setting(self, dbapi_connection): + return dbapi_connection.isolation_level is None + def on_connect(self): def regexp(a, b): if b is None: diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index ad0e4b6243..82729ee579 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -1112,10 +1112,16 @@ class Connection(ConnectionEventsTarget, inspection.Inspectable["Inspector"]): if self._still_open_and_dbapi_connection_is_valid: if self._echo: if self._is_autocommit_isolation(): - self._log_info( - "ROLLBACK using DBAPI connection.rollback(), " - "DBAPI should ignore due to autocommit mode" - ) + if self.dialect.skip_autocommit_rollback: + self._log_info( + "ROLLBACK will be skipped by " + "skip_autocommit_rollback" + ) + else: + self._log_info( + "ROLLBACK using DBAPI connection.rollback(); " + "set skip_autocommit_rollback to prevent fully" + ) else: self._log_info("ROLLBACK") try: @@ -1131,7 +1137,7 @@ class Connection(ConnectionEventsTarget, inspection.Inspectable["Inspector"]): if self._is_autocommit_isolation(): self._log_info( "COMMIT using DBAPI connection.commit(), " - "DBAPI should ignore due to autocommit mode" + "has no effect due to autocommit mode" ) else: self._log_info("COMMIT") diff --git a/lib/sqlalchemy/engine/create.py b/lib/sqlalchemy/engine/create.py index 920f620bd4..c0add3d8ea 100644 --- a/lib/sqlalchemy/engine/create.py +++ b/lib/sqlalchemy/engine/create.py @@ -171,6 +171,18 @@ def create_engine(url: Union[str, _url.URL], **kwargs: Any) -> Engine: :ref:`connections_toplevel` + :param skip_autocommit_rollback: When True, the dialect will + unconditionally skip all calls to the DBAPI ``connection.rollback()`` + method if the DBAPI connection is confirmed to be in "autocommit" mode. + The availability of this feature is dialect specific; if not available, + a ``NotImplementedError`` is raised by the dialect when rollback occurs. + + .. seealso:: + + :ref:`dbapi_autocommit_skip_rollback` + + .. versionadded:: 2.0.43 + :param connect_args: a dictionary of options which will be passed directly to the DBAPI's ``connect()`` method as additional keyword arguments. See the example @@ -468,6 +480,9 @@ def create_engine(url: Union[str, _url.URL], **kwargs: Any) -> Engine: :ref:`pool_reset_on_return` + :ref:`dbapi_autocommit_skip_rollback` - a more modern approach + to using connections with no transactional instructions + :param pool_timeout=30: number of seconds to wait before giving up on getting a connection from the pool. This is only used with :class:`~sqlalchemy.pool.QueuePool`. This can be a float but is diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py index 57759f79cf..a241e90115 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -307,6 +307,7 @@ class DefaultDialect(Dialect): # Linting.NO_LINTING constant compiler_linting: Linting = int(compiler.NO_LINTING), # type: ignore server_side_cursors: bool = False, + skip_autocommit_rollback: bool = False, **kwargs: Any, ): if server_side_cursors: @@ -331,6 +332,8 @@ class DefaultDialect(Dialect): self.dbapi = dbapi + self.skip_autocommit_rollback = skip_autocommit_rollback + if paramstyle is not None: self.paramstyle = paramstyle elif self.dbapi is not None: @@ -701,6 +704,10 @@ class DefaultDialect(Dialect): pass def do_rollback(self, dbapi_connection): + if self.skip_autocommit_rollback and self.detect_autocommit_setting( + dbapi_connection + ): + return dbapi_connection.rollback() def do_commit(self, dbapi_connection): diff --git a/lib/sqlalchemy/engine/interfaces.py b/lib/sqlalchemy/engine/interfaces.py index fd99afafd0..25e8d72bc9 100644 --- a/lib/sqlalchemy/engine/interfaces.py +++ b/lib/sqlalchemy/engine/interfaces.py @@ -779,6 +779,14 @@ class Dialect(EventTarget): default_isolation_level: Optional[IsolationLevel] """the isolation that is implicitly present on new connections""" + skip_autocommit_rollback: bool + """Whether or not the :paramref:`.create_engine.skip_autocommit_rollback + parameter was set. + + .. versionadded:: 2.0.43 + + """ + # create_engine() -> isolation_level currently goes here _on_connect_isolation_level: Optional[IsolationLevel] @@ -2481,6 +2489,30 @@ class Dialect(EventTarget): raise NotImplementedError() + def detect_autocommit_setting(self, dbapi_conn: DBAPIConnection) -> bool: + """Detect the current autocommit setting for a DBAPI connection. + + :param dbapi_connection: a DBAPI connection object + :return: True if autocommit is enabled, False if disabled + :rtype: bool + + This method inspects the given DBAPI connection to determine + whether autocommit mode is currently enabled. The specific + mechanism for detecting autocommit varies by database dialect + and DBAPI driver, however it should be done **without** network + round trips. + + .. note:: + + Not all dialects support autocommit detection. Dialects + that do not support this feature will raise + :exc:`NotImplementedError`. + + """ + raise NotImplementedError( + "This dialect cannot detect autocommit on a DBAPI connection" + ) + def get_default_isolation_level( self, dbapi_conn: DBAPIConnection ) -> IsolationLevel: diff --git a/lib/sqlalchemy/testing/requirements.py b/lib/sqlalchemy/testing/requirements.py index bd2dfd402f..b717cfc8ea 100644 --- a/lib/sqlalchemy/testing/requirements.py +++ b/lib/sqlalchemy/testing/requirements.py @@ -1033,6 +1033,13 @@ class SuiteRequirements(Requirements): """target dialect supports 'AUTOCOMMIT' as an isolation_level""" return exclusions.closed() + @property + def skip_autocommit_rollback(self): + """target dialect supports the detect_autocommit_setting() method and + uses the default implementation of do_rollback()""" + + return exclusions.closed() + @property def isolation_level(self): """target dialect supports general isolation level settings. diff --git a/lib/sqlalchemy/testing/suite/test_dialect.py b/lib/sqlalchemy/testing/suite/test_dialect.py index ebbb9e435a..36b474d53e 100644 --- a/lib/sqlalchemy/testing/suite/test_dialect.py +++ b/lib/sqlalchemy/testing/suite/test_dialect.py @@ -17,6 +17,7 @@ from .. import eq_ from .. import fixtures from .. import is_not_none from .. import is_true +from .. import mock from .. import ne_ from .. import provide_metadata from ..assertions import expect_raises @@ -293,7 +294,11 @@ class AutocommitIsolationTest(fixtures.TablesTest): test_needs_acid=True, ) - def _test_conn_autocommits(self, conn, autocommit): + def _test_conn_autocommits(self, conn, autocommit, ensure_table=False): + if ensure_table: + self.tables.some_table.create(conn, checkfirst=True) + conn.commit() + trans = conn.begin() conn.execute( self.tables.some_table.insert(), {"id": 1, "data": "some data"} @@ -336,6 +341,37 @@ class AutocommitIsolationTest(fixtures.TablesTest): ) self._test_conn_autocommits(conn, False) + @testing.requires.skip_autocommit_rollback + @testing.variation("autocommit_setting", ["false", "engine", "option"]) + @testing.variation("block_rollback", [True, False]) + def test_autocommit_block( + self, testing_engine, autocommit_setting, block_rollback + ): + kw = {} + if bool(block_rollback): + kw["skip_autocommit_rollback"] = True + if autocommit_setting.engine: + kw["isolation_level"] = "AUTOCOMMIT" + + engine = testing_engine(options=kw) + + conn = engine.connect() + if autocommit_setting.option: + conn.execution_options(isolation_level="AUTOCOMMIT") + self._test_conn_autocommits( + conn, + autocommit_setting.engine or autocommit_setting.option, + ensure_table=True, + ) + with mock.patch.object( + conn.connection, "rollback", wraps=conn.connection.rollback + ) as check_rollback: + conn.close() + if autocommit_setting.false or not block_rollback: + eq_(check_rollback.mock_calls, [mock.call()]) + else: + eq_(check_rollback.mock_calls, []) + @testing.requires.independent_readonly_connections @testing.variation("use_dialect_setting", [True, False]) def test_dialect_autocommit_is_restored( diff --git a/test/engine/test_logging.py b/test/engine/test_logging.py index 119d553320..d62d985d38 100644 --- a/test/engine/test_logging.py +++ b/test/engine/test_logging.py @@ -753,14 +753,14 @@ class TransactionContextLoggingTest(fixtures.TestBase): @testing.fixture() def logging_engine(self, testing_engine): - kw = {"echo": True, "future": True} + kw = {"echo": True} e = testing_engine(options=kw) e.connect().close() return e @testing.fixture() def autocommit_iso_logging_engine(self, testing_engine): - kw = {"echo": True, "future": True, "isolation_level": "AUTOCOMMIT"} + kw = {"echo": True, "isolation_level": "AUTOCOMMIT"} e = testing_engine(options=kw) e.connect().close() return e @@ -811,8 +811,8 @@ class TransactionContextLoggingTest(fixtures.TestBase): [ "BEGIN (implicit; DBAPI should not " "BEGIN due to autocommit mode)", - "COMMIT using DBAPI connection.commit(), DBAPI " - "should ignore due to autocommit mode", + "COMMIT using DBAPI connection.commit(), " + "has no effect due to autocommit mode", ] ) @@ -845,28 +845,45 @@ class TransactionContextLoggingTest(fixtures.TestBase): [ "BEGIN (implicit; DBAPI should not " "BEGIN due to autocommit mode)", - "COMMIT using DBAPI connection.commit(), DBAPI " - "should ignore due to autocommit mode", + "COMMIT using DBAPI connection.commit(), " + "has no effect due to autocommit mode", ] ) + @testing.variation("block_rollback", [True, False]) def test_commit_as_you_go_block_rollback_autocommit( - self, logging_engine, assert_buf + self, testing_engine, assert_buf, block_rollback ): - with logging_engine.connect().execution_options( - isolation_level="AUTOCOMMIT" - ) as conn: + + kw = { + "echo": True, + "isolation_level": "AUTOCOMMIT", + "skip_autocommit_rollback": bool(block_rollback), + } + logging_engine = testing_engine(options=kw) + logging_engine.connect().close() + + with logging_engine.connect() as conn: conn.begin() conn.rollback() - assert_buf( - [ - "BEGIN (implicit; DBAPI should not " - "BEGIN due to autocommit mode)", - "ROLLBACK using DBAPI connection.rollback(), DBAPI " - "should ignore due to autocommit mode", - ] - ) + if block_rollback: + assert_buf( + [ + "BEGIN (implicit; DBAPI should not " + "BEGIN due to autocommit mode)", + "ROLLBACK will be skipped by skip_autocommit_rollback", + ] + ) + else: + assert_buf( + [ + "BEGIN (implicit; DBAPI should not " + "BEGIN due to autocommit mode)", + "ROLLBACK using DBAPI connection.rollback(); " + "set skip_autocommit_rollback to prevent fully", + ] + ) def test_logging_compatibility( self, plain_assert_buf, plain_logging_engine diff --git a/test/requirements.py b/test/requirements.py index a770ea3fa0..ba481fb4ef 100644 --- a/test/requirements.py +++ b/test/requirements.py @@ -447,6 +447,14 @@ class DefaultRequirements(SuiteRequirements): in self.get_isolation_levels(config)["supported"] ) + @property + def skip_autocommit_rollback(self): + return exclusions.skip_if( + ["mssql+pymssql"], + "DBAPI has no means of testing the autocommit status of a " + "connection", + ) + @property def row_triggers(self): """Target must support standard statement-running EACH ROW triggers."""