fetchone() and others should not raise on a result that had rows

[Mike Bayer]

the resultproxy does an "autoclose", but fetchone() and others should continue to return None or empty list, the same way pep 249 states.

>>> from sqlalchemy import create_engine
>>> e = create_engine("sqlite://")
>>> r = e.execute("select 1")
>>> r.fetchone()
(1,)
>>> r.fetchone()
>>> r.fetchone()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/engine/result.py", line 930, in fetchone
    self.cursor, self.context)
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/engine/base.py", line 1335, in _handle_dbapi_exception
    util.reraise(*exc_info)
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/engine/result.py", line 921, in fetchone
    row = self._fetchone_impl()
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/engine/result.py", line 840, in _fetchone_impl
    self._non_result()
  File "/Users/classic/dev/sqlalchemy/lib/sqlalchemy/engine/result.py", line 864, in _non_result
    raise exc.ResourceClosedError("This result object is closed.")
sqlalchemy.exc.ResourceClosedError: This result object is closed.
>>> 

[Mike Bayer]

we should call it "_soft_close()" and keep it separate from .close().

[Mike Bayer]

here's a patch which also addresses #3329, however we might want 3329 in 0.9:

diff --git a/lib/sqlalchemy/engine/result.py b/lib/sqlalchemy/engine/result.py
index 3c31ae1..74fe2c3 100644
--- a/lib/sqlalchemy/engine/result.py
+++ b/lib/sqlalchemy/engine/result.py
@@ -620,6 +620,16 @@ class ResultProxy(object):

         return self._saved_cursor.description

+    def _soft_close(self, _autoclose_connection=True):
+        if self.cursor is None:
+            return
+        cursor = self.cursor
+        self.connection._safe_close_cursor(cursor)
+        if _autoclose_connection and \
+                self.connection.should_close_with_result:
+            self.connection.close()
+        self.cursor = None
+
     def close(self, _autoclose_connection=True):
         """Close this ResultProxy.

@@ -641,12 +651,7 @@ class ResultProxy(object):

         if not self.closed:
             self.closed = True
-            self.connection._safe_close_cursor(self.cursor)
-            if _autoclose_connection and \
-                    self.connection.should_close_with_result:
-                self.connection.close()
-            # allow consistent errors
-            self.cursor = None
+            self._soft_close(_autoclose_connection)

     def __iter__(self):
         while True:
@@ -837,7 +842,7 @@ class ResultProxy(object):
         try:
             return self.cursor.fetchone()
         except AttributeError:
-            self._non_result()
+            return self._non_result(None)

     def _fetchmany_impl(self, size=None):
         try:
@@ -846,22 +851,24 @@ class ResultProxy(object):
             else:
                 return self.cursor.fetchmany(size)
         except AttributeError:
-            self._non_result()
+            return self._non_result([])

     def _fetchall_impl(self):
         try:
             return self.cursor.fetchall()
         except AttributeError:
-            self._non_result()
+            return self._non_result([])

-    def _non_result(self):
+    def _non_result(self, default):
         if self._metadata is None:
             raise exc.ResourceClosedError(
                 "This result object does not return rows. "
                 "It has been closed automatically.",
             )
-        else:
+        elif self.closed:
             raise exc.ResourceClosedError("This result object is closed.")
+        else:
+            return default

     def process_rows(self, rows):
         process_row = self._process_row
@@ -884,7 +891,7 @@ class ResultProxy(object):

         try:
             l = self.process_rows(self._fetchall_impl())
-            self.close()
+            self._soft_close()
             return l
         except Exception as e:
             self.connection._handle_dbapi_exception(
@@ -903,7 +910,7 @@ class ResultProxy(object):
         try:
             l = self.process_rows(self._fetchmany_impl(size))
             if len(l) == 0:
-                self.close()
+                self._soft_close()
             return l
         except Exception as e:
             self.connection._handle_dbapi_exception(
@@ -922,7 +929,7 @@ class ResultProxy(object):
             if row is not None:
                 return self.process_rows([row])[0]
             else:
-                self.close()
+                self._soft_close()
                 return None
         except Exception as e:
             self.connection._handle_dbapi_exception(
@@ -936,7 +943,7 @@ class ResultProxy(object):

         """
         if self._metadata is None:
-            self._non_result()
+            return self._non_result(None)

         try:
             row = self._fetchone_impl()
@@ -951,7 +958,7 @@ class ResultProxy(object):
             else:
                 return None
         finally:
-            self.close()
+            self._soft_close()

     def scalar(self):
         """Fetch the first column of the first row, and close the result set.
diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py
index 730ef44..a5318c0 100644
--- a/test/engine/test_execute.py
+++ b/test/engine/test_execute.py
@@ -1070,6 +1070,9 @@ class AlternateResultProxyTest(fixtures.TestBase):
         rows = r.fetchmany(6)
         eq_(rows, [(i, "t_%d" % i) for i in range(1, 6)])

+        rows = r.fetchmany(2)
+        eq_(rows, [])
+
     def test_plain(self):
         self._test_proxy(_result.ResultProxy)

diff --git a/test/sql/test_query.py b/test/sql/test_query.py
index eeec487..08afc32 100644
--- a/test/sql/test_query.py
+++ b/test/sql/test_query.py
@@ -993,6 +993,9 @@ class QueryTest(fixtures.TestBase):
     def test_fetchone_til_end(self):
         result = testing.db.execute("select * from query_users")
         eq_(result.fetchone(), None)
+        eq_(result.fetchone(), None)
+        eq_(result.fetchone(), None)
+        result.close()
         assert_raises_message(
             exc.ResourceClosedError,
             "This result object is closed.",

[Mike Bayer]

• The "auto close" for :class:.ResultProxy is now a "soft" close. That is, after exhausing all rows using the fetch methods, the DBAPI cursor is released as before and the object may be safely discarded, but the fetch methods may continue to be called for which they will return an end-of-result object (None for fetchone, empty list for fetchmany and fetchall). Only if :meth:.ResultProxy.close is called explicitly will these methods raise the "result is closed" error. fixes #3330 fixes #3329

→ <<cset b7e151ac5cf5>>