fetchone() and others should not raise on a result that had rows
Issue #3330
resolved
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.
>>>
Comments (3)
-
reporter -
reporter 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.",
-
reporter - changed status to resolved
- 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#3330fixes#3329
→ <<cset b7e151ac5cf5>>
- Log in to comment
we should call it "_soft_close()" and keep it separate from .close().