ResultProxy won't autoclose connection until state flag is set

Changed the mechanics of :class:`.ResultProxy` to unconditionally
delay the "autoclose" step until the :class:`.Connection` is done
with the object; in the case where Postgresql ON CONFLICT with
RETURNING returns no rows, autoclose was occurring in this previously
non-existent use case, causing the usual autocommit behavior that
occurs unconditionally upon INSERT/UPDATE/DELETE to fail.

Change-Id: I235a25daf4381b31f523331f810ea04450349722
Fixes: #3955
(cherry picked from commit 8ee363e4917b0dcd64a83b6d26e465c9e61e0ea5)

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 7a53540a5d433d61a7370ac4ace25f3fbb7a5236..3a5b29decbc227f60f910b741aaa0063777201b5 100644 (file)
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -34,6 +34,17 @@
         left-hand side type to be transferred directly to the right hand side
         so that bind-level rules can be applied to the expression's argument.
 
+    .. change:: 3955
+        :tags: bug, sql, postgresql
+        :versions: 1.2.0b1
+        :tickets: 3955
+
+        Changed the mechanics of :class:`.ResultProxy` to unconditionally
+        delay the "autoclose" step until the :class:`.Connection` is done
+        with the object; in the case where Postgresql ON CONFLICT with
+        RETURNING returns no rows, autoclose was occurring in this previously
+        non-existent use case, causing the usual autocommit behavior that
+        occurs unconditionally upon INSERT/UPDATE/DELETE to fail.
 
 .. changelog::
     :version: 1.1.8

diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index f680edadaa5825c67d1458bc8d15c70e5a4695aa..91f4493c2e775471bbdbb91392fcd03527344f90 100644 (file)
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -1203,14 +1203,22 @@ class Connection(Connectable):
         else:
             result = context.get_result_proxy()
             if result._metadata is None:
-                result._soft_close(_autoclose_connection=False)
+                result._soft_close()
 
         if context.should_autocommit and self._root.__transaction is None:
             self._root._commit_impl(autocommit=True)
 
-        if result._soft_closed and self.should_close_with_result:
-            self.close()
-
+        # for "connectionless" execution, we have to close this
+        # Connection after the statement is complete.
+        if self.should_close_with_result:
+            # ResultProxy already exhausted rows / has no rows.
+            # close us now
+            if result._soft_closed:
+                self.close()
+            else:
+                # ResultProxy will close this Connection when no more
+                # rows to fetch.
+                result._autoclose_connection = True
         return result
 
     def _cursor_execute(self, cursor, statement, parameters, context=None):

diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py
index 18c3276f8e8d403b4fe924674583e951455e667f..bcc78be164751d7cf4b7b5c426299c81d9f59f58 100644 (file)
--- a/lib/sqlalchemy/engine/default.py
+++ b/lib/sqlalchemy/engine/default.py
@@ -905,15 +905,15 @@ class DefaultExecutionContext(interfaces.ExecutionContext):
                 row = result.fetchone()
                 self.returned_defaults = row
                 self._setup_ins_pk_from_implicit_returning(row)
-                result._soft_close(_autoclose_connection=False)
+                result._soft_close()
                 result._metadata = None
             elif not self._is_explicit_returning:
-                result._soft_close(_autoclose_connection=False)
+                result._soft_close()
                 result._metadata = None
         elif self.isupdate and self._is_implicit_returning:
             row = result.fetchone()
             self.returned_defaults = row
-            result._soft_close(_autoclose_connection=False)
+            result._soft_close()
             result._metadata = None
 
         elif result._metadata is None:
@@ -921,7 +921,7 @@ class DefaultExecutionContext(interfaces.ExecutionContext):
             # (which requires open cursor on some drivers
             # such as kintersbasdb, mxodbc)
             result.rowcount
-            result._soft_close(_autoclose_connection=False)
+            result._soft_close()
         return result
 
     def _setup_ins_pk_from_lastrowid(self):

diff --git a/lib/sqlalchemy/engine/result.py b/lib/sqlalchemy/engine/result.py
index 903b2c2f9ae75a87d0bc3370710460effe2db05a..907dc7bd29316fad892804ddd6d0ef814f961e9a 100644 (file)
--- a/lib/sqlalchemy/engine/result.py
+++ b/lib/sqlalchemy/engine/result.py
@@ -638,7 +638,7 @@ class ResultProxy(object):
 
     _process_row = RowProxy
     out_parameters = None
-    _can_close_connection = False
+    _autoclose_connection = False
     _metadata = None
     _soft_closed = False
     closed = False
@@ -792,7 +792,7 @@ class ResultProxy(object):
 
         return self._saved_cursor.description
 
-    def _soft_close(self, _autoclose_connection=True):
+    def _soft_close(self):
         """Soft close this :class:`.ResultProxy`.
 
         This releases all DBAPI cursor resources, but leaves the
@@ -820,8 +820,7 @@ class ResultProxy(object):
         self._soft_closed = True
         cursor = self.cursor
         self.connection._safe_close_cursor(cursor)
-        if _autoclose_connection and \
-                self.connection.should_close_with_result:
+        if self._autoclose_connection:
             self.connection.close()
         self.cursor = None
 

diff --git a/test/dialect/postgresql/test_on_conflict.py b/test/dialect/postgresql/test_on_conflict.py
index 7c83f2826229966c21a9f6a1d9ff22ca2ceae4a2..c3e1b91584ac122e409dcbf85fc35f36715489fb 100644 (file)
--- a/test/dialect/postgresql/test_on_conflict.py
+++ b/test/dialect/postgresql/test_on_conflict.py
@@ -12,6 +12,7 @@ class OnConflictTest(fixtures.TablesTest):
 
     __only_on__ = 'postgresql >= 9.5',
     __backend__ = True
+    run_define_tables = 'each'
 
     @classmethod
     def define_tables(cls, metadata):
@@ -79,6 +80,7 @@ class OnConflictTest(fixtures.TablesTest):
         with testing.db.connect() as conn:
             result = conn.execute(
                 insert(users).on_conflict_do_nothing(),
+
                 dict(id=1, name='name1')
             )
             eq_(result.inserted_primary_key, [1])
@@ -96,6 +98,33 @@ class OnConflictTest(fixtures.TablesTest):
                 [(1, 'name1')]
             )
 
+    def test_on_conflict_do_nothing_connectionless(self):
+        users = self.tables.users_xtra
+
+        with testing.db.connect() as conn:
+            result = conn.execute(
+                insert(users).on_conflict_do_nothing(
+                    constraint='uq_login_email'),
+
+                dict(name='name1', login_email='email1')
+            )
+            eq_(result.inserted_primary_key, [1])
+            eq_(result.returned_defaults, (1,))
+
+        result = testing.db.execute(
+            insert(users).on_conflict_do_nothing(
+                constraint='uq_login_email'
+            ),
+            dict(name='name2', login_email='email1')
+        )
+        eq_(result.inserted_primary_key, None)
+        eq_(result.returned_defaults, None)
+
+        eq_(
+            testing.db.execute(users.select().where(users.c.id == 1)).fetchall(),
+            [(1, 'name1', 'email1', None)]
+        )
+
     @testing.provide_metadata
     def test_on_conflict_do_nothing_target(self):
         users = self.tables.users

diff --git a/test/sql/test_resultset.py b/test/sql/test_resultset.py
index de677be9a4e0215c503cf74b44097112be54c100..48fe288613f8c751a1e2b15b77819586d49be508 100644 (file)
--- a/test/sql/test_resultset.py
+++ b/test/sql/test_resultset.py
@@ -489,6 +489,50 @@ class ResultProxyTest(fixtures.TablesTest):
             result.fetchone
         )
 
+    def test_connectionless_autoclose_rows_exhausted(self):
+        users = self.tables.users
+        users.insert().execute(
+            dict(user_id=1, user_name='john'),
+        )
+
+        result = testing.db.execute("select * from users")
+        connection = result.connection
+        assert not connection.closed
+        eq_(result.fetchone(), (1, 'john'))
+        assert not connection.closed
+        eq_(result.fetchone(), None)
+        assert connection.closed
+
+    @testing.requires.returning
+    def test_connectionless_autoclose_crud_rows_exhausted(self):
+        users = self.tables.users
+        stmt = users.insert().values(user_id=1, user_name='john').\
+            returning(users.c.user_id)
+        result = testing.db.execute(stmt)
+        connection = result.connection
+        assert not connection.closed
+        eq_(result.fetchone(), (1, ))
+        assert not connection.closed
+        eq_(result.fetchone(), None)
+        assert connection.closed
+
+    def test_connectionless_autoclose_no_rows(self):
+        result = testing.db.execute("select * from users")
+        connection = result.connection
+        assert not connection.closed
+        eq_(result.fetchone(), None)
+        assert connection.closed
+
+    def test_connectionless_autoclose_no_metadata(self):
+        result = testing.db.execute("update users set user_id=5")
+        connection = result.connection
+        assert connection.closed
+        assert_raises_message(
+            exc.ResourceClosedError,
+            "This result object does not return rows.",
+            result.fetchone
+        )
+
     def test_row_case_sensitive(self):
         row = testing.db.execute(
             select([