From: Matthew Wilkes <git@matthewwilkes.name>
Date: Thu, 9 May 2019 22:04:35 +0000 (-0400)
Subject: Move initialize do_rollback() outside of the dialect
X-Git-Tag: rel_1_3_4~19
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9a4e675e055313f2f76fdbf59e1836a158694621;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Move initialize do_rollback() outside of the dialect

Moved the "rollback" which occurs during dialect initialization so that it
occurs after additional dialect-specific initialize steps, in particular
those of the psycopg2 dialect which would inadvertently leave transactional
state on the first new connection, which could interfere with some
psycopg2-specific APIs which require that no transaction is started.  Pull
request courtesy Matthew Wilkes.

Fixes: #4663
Closes: #4664
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/4664
Pull-request-sha: e544fe671d443ed06b210ba1cd1d7ba9c5653831

Change-Id: If40a15a1679b4eec0b8b8222f678697728009c30
(cherry picked from commit f601791a914d3181252493800871c458ad6c46d1)
---

diff --git a/doc/build/changelog/unreleased_13/4663.rst b/doc/build/changelog/unreleased_13/4663.rst
new file mode 100644
index 0000000000..07b943e55c
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4663.rst
@@ -0,0 +1,11 @@
+.. change::
+   :tags: bug, engine, postgresql
+   :tickets: 4663
+
+   Moved the "rollback" which occurs during dialect initialization so that it
+   occurs after additional dialect-specific initialize steps, in particular
+   those of the psycopg2 dialect which would inadvertently leave transactional
+   state on the first new connection, which could interfere with some
+   psycopg2-specific APIs which require that no transaction is started.  Pull
+   request courtesy Matthew Wilkes.
+
diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py
index 51e2c4603b..f6c30cbf47 100644
--- a/lib/sqlalchemy/engine/default.py
+++ b/lib/sqlalchemy/engine/default.py
@@ -312,8 +312,6 @@ class DefaultDialect(interfaces.Dialect):
         ):
             self._description_decoder = self.description_encoding = None
 
-        self.do_rollback(connection.connection)
-
     def on_connect(self):
         """return a callable which sets up a newly created DBAPI connection.
 
diff --git a/lib/sqlalchemy/engine/strategies.py b/lib/sqlalchemy/engine/strategies.py
index e367ef8904..d3a22e5ac8 100644
--- a/lib/sqlalchemy/engine/strategies.py
+++ b/lib/sqlalchemy/engine/strategies.py
@@ -197,6 +197,7 @@ class DefaultEngineStrategy(EngineStrategy):
                 )
                 c._execution_options = util.immutabledict()
                 dialect.initialize(c)
+                dialect.do_rollback(c.connection)
 
             event.listen(pool, "first_connect", first_connect, once=True)
 
diff --git a/test/dialect/postgresql/test_dialect.py b/test/dialect/postgresql/test_dialect.py
index c68af2abb8..25cba6269c 100644
--- a/test/dialect/postgresql/test_dialect.py
+++ b/test/dialect/postgresql/test_dialect.py
@@ -39,6 +39,7 @@ from sqlalchemy.testing.assertions import assert_raises_message
 from sqlalchemy.testing.assertions import AssertsCompiledSQL
 from sqlalchemy.testing.assertions import AssertsExecutionResults
 from sqlalchemy.testing.assertions import eq_
+from sqlalchemy.testing.assertions import ne_
 from sqlalchemy.testing.mock import Mock
 from ...engine import test_execute
 
@@ -500,6 +501,14 @@ class MiscBackendTest(
                 "c %s NOT NULL" % expected,
             )
 
+    @testing.requires.psycopg2_compatibility
+    def test_initial_transaction_state(self):
+        from psycopg2.extensions import STATUS_IN_TRANSACTION
+
+        engine = engines.testing_engine()
+        with engine.connect() as conn:
+            ne_(conn.connection.status, STATUS_IN_TRANSACTION)
+
 
 class AutocommitTextTest(test_execute.AutocommitTextTest):
     __only_on__ = "postgresql"
diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py
index e18cdfad4c..480da71221 100644
--- a/test/engine/test_execute.py
+++ b/test/engine/test_execute.py
@@ -660,6 +660,15 @@ class ExecuteTest(fixtures.TestBase):
         eq_(conn._execution_options, {"autocommit": True})
         conn.close()
 
+    def test_initialize_rollback(self):
+        """test a rollback happens during first connect"""
+        eng = create_engine(testing.db.url)
+        with patch.object(eng.dialect, "do_rollback") as do_rollback:
+            assert do_rollback.call_count == 0
+            connection = eng.connect()
+            assert do_rollback.call_count == 1
+        connection.close()
+
     @testing.requires.ad_hoc_engines
     def test_dialect_init_uses_options(self):
         eng = create_engine(testing.db.url)