connectionless execution + autocommit + ON CONFLICT DO NOTHING + no rows == boom

[sqlalchemy-bot]

Migrated issue, originally created by Michael Bayer (@zzzeek)

@@ -96,6 +97,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

the lack of row returned sends the connection into autoclose via result.fetchone(). then autocommit causes an exception, then it continues to be confused because the connection has been closed.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

so this is the first approach, not sure if I like this because I'd perhaps like to see COMMIT even though the RETURNING had no rows:

diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index f680eda..8f33fe1 100644
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -1205,7 +1205,9 @@ class Connection(Connectable):
             if result._metadata is None:
                 result._soft_close(_autoclose_connection=False)
 
-        if context.should_autocommit and self._root.__transaction is None:
+        if context.should_autocommit and \
+                self._root.__transaction is None and \
+                not self.closed:
             self._root._commit_impl(autocommit=True)
 
         if result._soft_closed and self.should_close_with_result:

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

https://gerrit.sqlalchemy.org/#/q/I235a25daf4381b31f523331f810ea04450349722

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

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)

→ f52fb52

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

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)
(cherry picked from commit f52fb52)

→ 9609f5f