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

Issue #3955 resolved
Mike Bayer repo owner created an issue
@@ -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.

Comments (4)

  1. Mike Bayer reporter

    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:
    
  2. Mike Bayer reporter

    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)

    → <<cset f52fb5282a04>>

  3. Mike Bayer reporter

    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 f52fb5282a046d26b6ee2778e03b995eb117c2ee)

    → <<cset 9609f5ffb52c>>

  4. Log in to comment