more autocommit messaging

Further clarified connection-level logging to indicate the BEGIN, ROLLBACK
and COMMIT log messages do not actually indicate a real transaction when
the AUTOCOMMIT isolation level is in use; messaging has been extended to
include the BEGIN message itself, and the messaging has also been fixed to
accommodate when the :class:`.Engine` level
:paramref:`.create_engine.isolation_level` parameter was used directly.

Fixes: #7853
Change-Id: Iafc78070737ad117f84262e4bde84b81a81e4ea1

diff --git a/doc/build/changelog/unreleased_14/7853.rst b/doc/build/changelog/unreleased_14/7853.rst
new file mode 100644 (file)
index 0000000..66856c2
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/7853.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, engine
+    :tickets: 7853
+
+    Further clarified connection-level logging to indicate the BEGIN, ROLLBACK
+    and COMMIT log messages do not actually indicate a real transaction when
+    the AUTOCOMMIT isolation level is in use; messaging has been extended to
+    include the BEGIN message itself, and the messaging has also been fixed to
+    accommodate when the :class:`.Engine` level
+    :paramref:`.create_engine.isolation_level` parameter was used directly.

diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index 061794bded92743f35980d430721020c7dbf8ad7..6a79cdf0263dd39080ec2b6a385eca75265b21e4 100644 (file)
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -925,9 +925,14 @@ class Connection(ConnectionEventsTarget, inspection.Inspectable["Inspector"]):
         )
 
     def _is_autocommit_isolation(self) -> bool:
+        opt_iso = self._execution_options.get("isolation_level", None)
         return bool(
-            self._execution_options.get("isolation_level", None)
-            == "AUTOCOMMIT"
+            opt_iso == "AUTOCOMMIT"
+            or (
+                opt_iso is None
+                and self.engine.dialect._on_connect_isolation_level
+                == "AUTOCOMMIT"
+            )
         )
 
     def get_transaction(self) -> Optional[RootTransaction]:
@@ -949,7 +954,13 @@ class Connection(ConnectionEventsTarget, inspection.Inspectable["Inspector"]):
 
     def _begin_impl(self, transaction: RootTransaction) -> None:
         if self._echo:
-            self._log_info("BEGIN (implicit)")
+            if self._is_autocommit_isolation():
+                self._log_info(
+                    "BEGIN (implicit; DBAPI should not BEGIN due to "
+                    "autocommit mode)"
+                )
+            else:
+                self._log_info("BEGIN (implicit)")
 
         self.__in_begin = True
 

diff --git a/lib/sqlalchemy/engine/interfaces.py b/lib/sqlalchemy/engine/interfaces.py
index 1b178641e1de660a38c534172c4511c8584e1ec5..aa75da61411a0636be262af7db6f3d6877bd1a9b 100644 (file)
--- a/lib/sqlalchemy/engine/interfaces.py
+++ b/lib/sqlalchemy/engine/interfaces.py
@@ -677,9 +677,14 @@ class Dialect(EventTarget):
 
     """
 
+    # NOTE: this does not take into effect engine-level isolation level.
+    # not clear if this should be changed, seems like it should
     default_isolation_level: Optional[_IsolationLevel]
     """the isolation that is implicitly present on new connections"""
 
+    # create_engine()  -> isolation_level  currently goes here
+    _on_connect_isolation_level: Optional[_IsolationLevel]
+
     execution_ctx_cls: Type["ExecutionContext"]
     """a :class:`.ExecutionContext` class used to handle statement execution"""
 

diff --git a/test/engine/test_logging.py b/test/engine/test_logging.py
index ff8c3fc56dc42c8eb1bff7efe6344b704c8cbc8c..c6fd8568471e1ca169cd5d68b6f83c996d60e852 100644 (file)
--- a/test/engine/test_logging.py
+++ b/test/engine/test_logging.py
@@ -625,6 +625,13 @@ class TransactionContextLoggingTest(fixtures.TestBase):
         e.connect().close()
         return e
 
+    @testing.fixture()
+    def autocommit_iso_logging_engine(self, testing_engine):
+        kw = {"echo": True, "future": True, "isolation_level": "AUTOCOMMIT"}
+        e = testing_engine(options=kw)
+        e.connect().close()
+        return e
+
     @testing.fixture()
     def plain_logging_engine(self, testing_engine):
         # deliver an engine with logging using the plain logging API,
@@ -660,6 +667,38 @@ class TransactionContextLoggingTest(fixtures.TestBase):
 
         assert_buf(["BEGIN (implicit)", "ROLLBACK"])
 
+    def test_commit_as_you_go_block_commit_engine_level_autocommit(
+        self, autocommit_iso_logging_engine, assert_buf
+    ):
+        with autocommit_iso_logging_engine.connect() as conn:
+            conn.begin()
+            conn.commit()
+
+        assert_buf(
+            [
+                "BEGIN (implicit; DBAPI should not "
+                "BEGIN due to autocommit mode)",
+                "COMMIT using DBAPI connection.commit(), DBAPI "
+                "should ignore due to autocommit mode",
+            ]
+        )
+
+    def test_commit_engine_level_autocommit_exec_opt_nonauto(
+        self, autocommit_iso_logging_engine, assert_buf
+    ):
+        with autocommit_iso_logging_engine.execution_options(
+            isolation_level=testing.db.dialect.default_isolation_level
+        ).connect() as conn:
+            conn.begin()
+            conn.commit()
+
+        assert_buf(
+            [
+                "BEGIN (implicit)",
+                "COMMIT",
+            ]
+        )
+
     def test_commit_as_you_go_block_commit_autocommit(
         self, logging_engine, assert_buf
     ):
@@ -671,7 +710,8 @@ class TransactionContextLoggingTest(fixtures.TestBase):
 
         assert_buf(
             [
-                "BEGIN (implicit)",
+                "BEGIN (implicit; DBAPI should not "
+                "BEGIN due to autocommit mode)",
                 "COMMIT using DBAPI connection.commit(), DBAPI "
                 "should ignore due to autocommit mode",
             ]
@@ -688,7 +728,8 @@ class TransactionContextLoggingTest(fixtures.TestBase):
 
         assert_buf(
             [
-                "BEGIN (implicit)",
+                "BEGIN (implicit; DBAPI should not "
+                "BEGIN due to autocommit mode)",
                 "ROLLBACK using DBAPI connection.rollback(), DBAPI "
                 "should ignore due to autocommit mode",
             ]