diff options
author | Mike Bayer <mike_mp@zzzcomputing.com> | 2015-03-17 12:32:33 -0400 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2015-03-17 12:32:33 -0400 |
commit | b7e151ac5cf5a0c13b9a30bc6841ed0cfe322536 (patch) | |
tree | 039e129fb13d3fbafd2dcc718c15a5a2ea85a49f | |
parent | 2cadd768aa48d1180e24600cf133586a343ea10b (diff) | |
download | sqlalchemy-b7e151ac5cf5a0c13b9a30bc6841ed0cfe322536.tar.gz |
- 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
-rw-r--r-- | doc/build/changelog/changelog_10.rst | 17 | ||||
-rw-r--r-- | doc/build/changelog/migration_10.rst | 52 | ||||
-rw-r--r-- | doc/build/core/connections.rst | 1 | ||||
-rw-r--r-- | lib/sqlalchemy/dialects/sqlite/base.py | 2 | ||||
-rw-r--r-- | lib/sqlalchemy/engine/base.py | 4 | ||||
-rw-r--r-- | lib/sqlalchemy/engine/default.py | 8 | ||||
-rw-r--r-- | lib/sqlalchemy/engine/result.py | 171 | ||||
-rw-r--r-- | lib/sqlalchemy/testing/suite/test_insert.py | 9 | ||||
-rw-r--r-- | test/engine/test_execute.py | 41 | ||||
-rw-r--r-- | test/sql/test_query.py | 3 |
10 files changed, 264 insertions, 44 deletions
diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index f8fd456f0..6d8aa67da 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -19,6 +19,23 @@ :version: 1.0.0b2 .. change:: + :tags: bug, engine + :tickets: 3330, 3329 + + 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. + + .. seealso:: + + :ref:`change_3330` + + .. change:: :tags: bug, orm :tickets: 3327 :pullreq: github:160 diff --git a/doc/build/changelog/migration_10.rst b/doc/build/changelog/migration_10.rst index 23d230e94..dfebae08d 100644 --- a/doc/build/changelog/migration_10.rst +++ b/doc/build/changelog/migration_10.rst @@ -635,6 +635,58 @@ required during a CREATE/DROP scenario. :ticket:`3282` +.. _change_3330: + +ResultProxy "auto close" is now a "soft" close +---------------------------------------------- + +For many releases, the :class:`.ResultProxy` object has always been +automatically closed out at the point at which all result rows have been +fetched. This was to allow usage of the object without the need to call +upon :meth:`.ResultProxy.close` explicitly; as all DBAPI resources had been +freed, the object was safe to discard. However, the object maintained +a strict "closed" behavior, which meant that any subsequent calls to +:meth:`.ResultProxy.fetchone`, :meth:`.ResultProxy.fetchmany` or +:meth:`.ResultProxy.fetchall` would now raise a :class:`.ResourceClosedError`:: + + >>> result = connection.execute(stmt) + >>> result.fetchone() + (1, 'x') + >>> result.fetchone() + None # indicates no more rows + >>> result.fetchone() + exception: ResourceClosedError + +This behavior is inconsistent vs. what pep-249 states, which is +that you can call upon the fetch methods repeatedly even after results +are exhausted. It also interferes with behavior for some implementations of +result proxy, such as the :class:`.BufferedColumnResultProxy` used by the +cx_oracle dialect for certain datatypes. + +To solve this, the "closed" state of the :class:`.ResultProxy` has been +broken into two states; a "soft close" which does the majority of what +"close" does, in that it releases the DBAPI cursor and in the case of a +"close with result" object will also release the connection, and a +"closed" state which is everything included by "soft close" as well as +establishing the fetch methods as "closed". The :meth:`.ResultProxy.close` +method is now never called implicitly, only the :meth:`.ResultProxy._soft_close` +method which is non-public:: + + >>> result = connection.execute(stmt) + >>> result.fetchone() + (1, 'x') + >>> result.fetchone() + None # indicates no more rows + >>> result.fetchone() + None # still None + >>> result.fetchall() + [] + >>> result.close() + >>> result.fetchone() + exception: ResourceClosedError # *now* it raises + +:ticket:`3330` +:ticket:`3329` CHECK Constraints now support the ``%(column_0_name)s`` token in naming conventions ----------------------------------------------------------------------------------- diff --git a/doc/build/core/connections.rst b/doc/build/core/connections.rst index 6d7e7622f..b6770bb82 100644 --- a/doc/build/core/connections.rst +++ b/doc/build/core/connections.rst @@ -599,6 +599,7 @@ Connection / Engine API .. autoclass:: ResultProxy :members: + :private-members: _soft_close .. autoclass:: RowProxy :members: diff --git a/lib/sqlalchemy/dialects/sqlite/base.py b/lib/sqlalchemy/dialects/sqlite/base.py index 0f32395e5..0254690b4 100644 --- a/lib/sqlalchemy/dialects/sqlite/base.py +++ b/lib/sqlalchemy/dialects/sqlite/base.py @@ -1305,7 +1305,7 @@ class SQLiteDialect(default.DefaultDialect): qtable = quote(table_name) statement = "%s%s(%s)" % (statement, pragma, qtable) cursor = connection.execute(statement) - if not cursor.closed: + if not cursor._soft_closed: # work around SQLite issue whereby cursor.description # is blank when PRAGMA returns no rows: # http://www.sqlite.org/cvstrac/tktview?tn=1884 diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index 6be446313..5921ab9ba 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -1160,12 +1160,12 @@ class Connection(Connectable): else: result = context.get_result_proxy() if result._metadata is None: - result.close(_autoclose_connection=False) + result._soft_close(_autoclose_connection=False) if context.should_autocommit and self._root.__transaction is None: self._root._commit_impl(autocommit=True) - if result.closed and self.should_close_with_result: + if result._soft_closed and self.should_close_with_result: self.close() return result diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py index b46acb650..3eebc6c06 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -815,15 +815,15 @@ class DefaultExecutionContext(interfaces.ExecutionContext): row = result.fetchone() self.returned_defaults = row self._setup_ins_pk_from_implicit_returning(row) - result.close(_autoclose_connection=False) + result._soft_close(_autoclose_connection=False) result._metadata = None elif not self._is_explicit_returning: - result.close(_autoclose_connection=False) + result._soft_close(_autoclose_connection=False) result._metadata = None elif self.isupdate and self._is_implicit_returning: row = result.fetchone() self.returned_defaults = row - result.close(_autoclose_connection=False) + result._soft_close(_autoclose_connection=False) result._metadata = None elif result._metadata is None: @@ -831,7 +831,7 @@ class DefaultExecutionContext(interfaces.ExecutionContext): # (which requires open cursor on some drivers # such as kintersbasdb, mxodbc) result.rowcount - result.close(_autoclose_connection=False) + result._soft_close(_autoclose_connection=False) return result def _setup_ins_pk_from_lastrowid(self): diff --git a/lib/sqlalchemy/engine/result.py b/lib/sqlalchemy/engine/result.py index 3c31ae1ea..6d19cb6d0 100644 --- a/lib/sqlalchemy/engine/result.py +++ b/lib/sqlalchemy/engine/result.py @@ -479,11 +479,12 @@ class ResultProxy(object): out_parameters = None _can_close_connection = False _metadata = None + _soft_closed = False + closed = False def __init__(self, context): self.context = context self.dialect = context.dialect - self.closed = False self.cursor = self._saved_cursor = context.cursor self.connection = context.root_connection self._echo = self.connection._echo and \ @@ -620,33 +621,79 @@ class ResultProxy(object): return self._saved_cursor.description - def close(self, _autoclose_connection=True): - """Close this ResultProxy. - - Closes the underlying DBAPI cursor corresponding to the execution. - - Note that any data cached within this ResultProxy is still available. - For some types of results, this may include buffered rows. + def _soft_close(self, _autoclose_connection=True): + """Soft close this :class:`.ResultProxy`. - If this ResultProxy was generated from an implicit execution, - the underlying Connection will also be closed (returns the - underlying DBAPI connection to the connection pool.) + This releases all DBAPI cursor resources, but leaves the + ResultProxy "open" from a semantic perspective, meaning the + fetchXXX() methods will continue to return empty results. This method is called automatically when: * all result rows are exhausted using the fetchXXX() methods. * cursor.description is None. + This method is **not public**, but is documented in order to clarify + the "autoclose" process used. + + .. versionadded:: 1.0.0 + + .. seealso:: + + :meth:`.ResultProxy.close` + + + """ + if self._soft_closed: + return + self._soft_closed = True + 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): + """Close this ResultProxy. + + This closes out the underlying DBAPI cursor corresonding + to the statement execution, if one is stil present. Note that the + DBAPI cursor is automatically released when the :class:`.ResultProxy` + exhausts all available rows. :meth:`.ResultProxy.close` is generally + an optional method except in the case when discarding a + :class:`.ResultProxy` that still has additional rows pending for fetch. + + In the case of a result that is the product of + :ref:`connectionless execution <dbengine_implicit>`, + the underyling :class:`.Connection` object is also closed, which + :term:`releases` DBAPI connection resources. + + After this method is called, it is no longer valid to call upon + the fetch methods, which will raise a :class:`.ResourceClosedError` + on subsequent use. + + .. versionchanged:: 1.0.0 - the :meth:`.ResultProxy.close` method + has been separated out from the process that releases the underlying + DBAPI cursor resource. The "auto close" feature of the + :class:`.Connection` now performs a so-called "soft close", which + releases the underlying DBAPI cursor, but allows the + :class:`.ResultProxy` to still behave as an open-but-exhausted + result set; the actual :meth:`.ResultProxy.close` method is never + called. It is still safe to discard a :class:`.ResultProxy` + that has been fully exhausted without calling this method. + + .. seealso:: + + :ref:`connections_toplevel` + + :meth:`.ResultProxy._soft_close` + """ if not self.closed: + self._soft_close() 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 def __iter__(self): while True: @@ -837,7 +884,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 +893,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 @@ -880,11 +929,25 @@ class ResultProxy(object): for row in rows] def fetchall(self): - """Fetch all rows, just like DB-API ``cursor.fetchall()``.""" + """Fetch all rows, just like DB-API ``cursor.fetchall()``. + + After all rows have been exhausted, the underlying DBAPI + cursor resource is released, and the object may be safely + discarded. + + Subsequent calls to :meth:`.ResultProxy.fetchall` will return + an empty list. After the :meth:`.ResultProxy.close` method is + called, the method will raise :class:`.ResourceClosedError`. + + .. versionchanged:: 1.0.0 - Added "soft close" behavior which + allows the result to be used in an "exhausted" state prior to + calling the :meth:`.ResultProxy.close` method. + + """ try: l = self.process_rows(self._fetchall_impl()) - self.close() + self._soft_close() return l except Exception as e: self.connection._handle_dbapi_exception( @@ -895,15 +958,25 @@ class ResultProxy(object): """Fetch many rows, just like DB-API ``cursor.fetchmany(size=cursor.arraysize)``. - If rows are present, the cursor remains open after this is called. - Else the cursor is automatically closed and an empty list is returned. + After all rows have been exhausted, the underlying DBAPI + cursor resource is released, and the object may be safely + discarded. + + Calls to :meth:`.ResultProxy.fetchmany` after all rows have been + exhuasted will return + an empty list. After the :meth:`.ResultProxy.close` method is + called, the method will raise :class:`.ResourceClosedError`. + + .. versionchanged:: 1.0.0 - Added "soft close" behavior which + allows the result to be used in an "exhausted" state prior to + calling the :meth:`.ResultProxy.close` method. """ 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( @@ -913,8 +986,18 @@ class ResultProxy(object): def fetchone(self): """Fetch one row, just like DB-API ``cursor.fetchone()``. - If a row is present, the cursor remains open after this is called. - Else the cursor is automatically closed and None is returned. + After all rows have been exhausted, the underlying DBAPI + cursor resource is released, and the object may be safely + discarded. + + Calls to :meth:`.ResultProxy.fetchone` after all rows have + been exhausted will return ``None``. + After the :meth:`.ResultProxy.close` method is + called, the method will raise :class:`.ResourceClosedError`. + + .. versionchanged:: 1.0.0 - Added "soft close" behavior which + allows the result to be used in an "exhausted" state prior to + calling the :meth:`.ResultProxy.close` method. """ try: @@ -922,7 +1005,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( @@ -934,9 +1017,12 @@ class ResultProxy(object): Returns None if no row is present. + After calling this method, the object is fully closed, + e.g. the :meth:`.ResultProxy.close` method will have been called. + """ if self._metadata is None: - self._non_result() + return self._non_result(None) try: row = self._fetchone_impl() @@ -958,6 +1044,9 @@ class ResultProxy(object): Returns None if no row is present. + After calling this method, the object is fully closed, + e.g. the :meth:`.ResultProxy.close` method will have been called. + """ row = self.first() if row is not None: @@ -1001,13 +1090,19 @@ class BufferedRowResultProxy(ResultProxy): } def __buffer_rows(self): + if self.cursor is None: + return size = getattr(self, '_bufsize', 1) self.__rowbuffer = collections.deque(self.cursor.fetchmany(size)) self._bufsize = self.size_growth.get(size, size) + def _soft_close(self, **kw): + self.__rowbuffer.clear() + super(BufferedRowResultProxy, self)._soft_close(**kw) + def _fetchone_impl(self): - if self.closed: - return None + if self.cursor is None: + return self._non_result(None) if not self.__rowbuffer: self.__buffer_rows() if not self.__rowbuffer: @@ -1026,6 +1121,8 @@ class BufferedRowResultProxy(ResultProxy): return result def _fetchall_impl(self): + if self.cursor is None: + return self._non_result([]) self.__rowbuffer.extend(self.cursor.fetchall()) ret = self.__rowbuffer self.__rowbuffer = collections.deque() @@ -1048,11 +1145,15 @@ class FullyBufferedResultProxy(ResultProxy): def _buffer_rows(self): return collections.deque(self.cursor.fetchall()) + def _soft_close(self, **kw): + self.__rowbuffer.clear() + super(FullyBufferedResultProxy, self)._soft_close(**kw) + def _fetchone_impl(self): if self.__rowbuffer: return self.__rowbuffer.popleft() else: - return None + return self._non_result(None) def _fetchmany_impl(self, size=None): if size is None: @@ -1066,6 +1167,8 @@ class FullyBufferedResultProxy(ResultProxy): return result def _fetchall_impl(self): + if not self.cursor: + return self._non_result([]) ret = self.__rowbuffer self.__rowbuffer = collections.deque() return ret diff --git a/lib/sqlalchemy/testing/suite/test_insert.py b/lib/sqlalchemy/testing/suite/test_insert.py index 38519dfb9..70e8a6b17 100644 --- a/lib/sqlalchemy/testing/suite/test_insert.py +++ b/lib/sqlalchemy/testing/suite/test_insert.py @@ -109,7 +109,8 @@ class InsertBehaviorTest(fixtures.TablesTest): self.tables.autoinc_pk.insert(), data="some data" ) - assert r.closed + assert r._soft_closed + assert not r.closed assert r.is_insert assert not r.returns_rows @@ -119,7 +120,8 @@ class InsertBehaviorTest(fixtures.TablesTest): self.tables.autoinc_pk.insert(), data="some data" ) - assert r.closed + assert r._soft_closed + assert not r.closed assert r.is_insert assert not r.returns_rows @@ -128,7 +130,8 @@ class InsertBehaviorTest(fixtures.TablesTest): r = config.db.execute( self.tables.autoinc_pk.insert(), ) - assert r.closed + assert r._soft_closed + assert not r.closed r = config.db.execute( self.tables.autoinc_pk.select(). diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py index 730ef4446..b0256d325 100644 --- a/test/engine/test_execute.py +++ b/test/engine/test_execute.py @@ -1070,6 +1070,47 @@ class AlternateResultProxyTest(fixtures.TestBase): rows = r.fetchmany(6) eq_(rows, [(i, "t_%d" % i) for i in range(1, 6)]) + # result keeps going just fine with blank results... + eq_(r.fetchmany(2), []) + + eq_(r.fetchmany(2), []) + + eq_(r.fetchall(), []) + + eq_(r.fetchone(), None) + + # until we close + r.close() + + self._assert_result_closed(r) + + r = self.engine.execute(select([self.table]).limit(5)) + eq_(r.first(), (1, "t_1")) + self._assert_result_closed(r) + + r = self.engine.execute(select([self.table]).limit(5)) + eq_(r.scalar(), 1) + self._assert_result_closed(r) + + def _assert_result_closed(self, r): + assert_raises_message( + tsa.exc.ResourceClosedError, + "object is closed", + r.fetchone + ) + + assert_raises_message( + tsa.exc.ResourceClosedError, + "object is closed", + r.fetchmany, 2 + ) + + assert_raises_message( + tsa.exc.ResourceClosedError, + "object is closed", + r.fetchall + ) + def test_plain(self): self._test_proxy(_result.ResultProxy) diff --git a/test/sql/test_query.py b/test/sql/test_query.py index eeec487be..08afc3256 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.", |