From: Matthew Wilkes <git@matthewwilkes.name>
Date: Mon, 6 May 2019 15:52:49 +0000 (+0100)
Subject: Fixes: #4663 : Move the explicit in `DefaultDialect` initialization to
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e544fe671d443ed06b210ba1cd1d7ba9c5653831;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Fixes: #4663 : Move the explicit in `DefaultDialect` initialization to
the pool's `first_connect` handler. This ensures that all dialects will
always roll back after completing their call to `initialize(conn)`.
This has the effect of ensuring that the first connection to a postgresql
database is in the same state as all subsequent connections, i.e. not with
an implicit transaction open from the feature detection queries.
---

diff --git a/doc/build/changelog/unreleased_14/4663.rst b/doc/build/changelog/unreleased_14/4663.rst
new file mode 100644
index 0000000000..8f214e7964
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/4663.rst
@@ -0,0 +1,7 @@
+.. change::
+   :tags: bug, orm, postgresql
+   :tickets: 4663
+
+   Added additional rollback during engine creation for postgresql
+   engines that prevents the first connection starting having an
+   implicit transaction created earlier than usual.
\ No newline at end of file
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..e49374bcb2 100644
--- a/test/dialect/postgresql/test_dialect.py
+++ b/test/dialect/postgresql/test_dialect.py
@@ -500,6 +500,12 @@ class MiscBackendTest(
                 "c %s NOT NULL" % expected,
             )
 
+    def test_initial_transaction_state(self):
+        from psycopg2.extensions import STATUS_IN_TRANSACTION
+        engine = engines.testing_engine()
+        with engine.connect() as conn:
+            assert 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)