From: Mike Bayer Date: Tue, 3 Aug 2010 15:55:12 +0000 (-0400) Subject: - Calling fetchone() or similar on a result that X-Git-Tag: rel_0_6_4~59 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ddf4210552fdff864a885b1ec6aa238105b16e8f;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Calling fetchone() or similar on a result that has already been exhausted, has been closed, or is not a result-returning result now raises ResourceClosedError, a subclass of InvalidRequestError, in all cases, regardless of backend. Previously, some DBAPIs would raise ProgrammingError (i.e. pysqlite), others would return None leading to downstream breakages (i.e. MySQL-python). - Connection, ResultProxy, as well as Session use ResourceClosedError for all "this connection/transaction/result is closed" types of errors. --- diff --git a/CHANGES b/CHANGES index 836ccf20f4..13d6e0f639 100644 --- a/CHANGES +++ b/CHANGES @@ -74,6 +74,21 @@ CHANGES of 64 for index names, separate from their overall max length of 255. [ticket:1412] + - Calling fetchone() or similar on a result that + has already been exhausted, has been closed, + or is not a result-returning result now + raises ResourceClosedError, a subclass of + InvalidRequestError, in all cases, regardless + of backend. Previously, some DBAPIs would + raise ProgrammingError (i.e. pysqlite), others + would return None leading to downstream breakages + (i.e. MySQL-python). + + - Connection, ResultProxy, as well as Session use + ResourceClosedError for all "this + connection/transaction/result is closed" types of + errors. + - declarative - if @classproperty is used with a regular class-bound mapper property attribute, it will be called to get the diff --git a/lib/sqlalchemy/dialects/sqlite/base.py b/lib/sqlalchemy/dialects/sqlite/base.py index 2d9e56baf2..b84b18e68e 100644 --- a/lib/sqlalchemy/dialects/sqlite/base.py +++ b/lib/sqlalchemy/dialects/sqlite/base.py @@ -600,5 +600,5 @@ def _pragma_cursor(cursor): """work around SQLite issue whereby cursor.description is blank when PRAGMA returns no rows.""" if cursor.closed: - cursor._fetchone_impl = lambda: None + cursor.fetchone = lambda: None return cursor diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index cf459f9e65..684df51ca7 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -908,7 +908,7 @@ class Connection(Connectable): self.__connection = self.engine.raw_connection() self.__invalid = False return self.__connection - raise exc.InvalidRequestError("This Connection is closed") + raise exc.ResourceClosedError("This Connection is closed") @property def info(self): @@ -956,7 +956,7 @@ class Connection(Connectable): """ if self.closed: - raise exc.InvalidRequestError("This Connection is closed") + raise exc.ResourceClosedError("This Connection is closed") if self.__connection.is_valid: self.__connection.invalidate(exception) @@ -2279,7 +2279,9 @@ class ResultProxy(object): if _autoclose_connection and \ self.connection.should_close_with_result: self.connection.close() - + # allow consistent errors + self.cursor = None + def __iter__(self): while True: row = self.fetchone() @@ -2361,14 +2363,32 @@ class ResultProxy(object): return self.dialect.supports_sane_multi_rowcount def _fetchone_impl(self): - return self.cursor.fetchone() + try: + return self.cursor.fetchone() + except AttributeError: + self._non_result() def _fetchmany_impl(self, size=None): - return self.cursor.fetchmany(size) + try: + return self.cursor.fetchmany(size) + except AttributeError: + self._non_result() def _fetchall_impl(self): - return self.cursor.fetchall() - + try: + return self.cursor.fetchall() + except AttributeError: + self._non_result() + + def _non_result(self): + if self._metadata is None: + raise exc.ResourceClosedError( + "This result object does not return rows. " + "It has been closed automatically.", + ) + else: + raise exc.ResourceClosedError("This result object is closed.") + def process_rows(self, rows): process_row = self._process_row metadata = self._metadata @@ -2425,7 +2445,6 @@ class ResultProxy(object): Else the cursor is automatically closed and None is returned. """ - try: row = self._fetchone_impl() if row is not None: @@ -2445,6 +2464,9 @@ class ResultProxy(object): Returns None if no row is present. """ + if self._metadata is None: + self._non_result() + try: row = self._fetchone_impl() except Exception, e: diff --git a/lib/sqlalchemy/exc.py b/lib/sqlalchemy/exc.py index 31826f44f8..1c412824cc 100644 --- a/lib/sqlalchemy/exc.py +++ b/lib/sqlalchemy/exc.py @@ -60,6 +60,10 @@ class InvalidRequestError(SQLAlchemyError): """ +class ResourceClosedError(InvalidRequestError): + """An operation was requested from a connection, cursor, or other + object that's in a closed state.""" + class NoSuchColumnError(KeyError, InvalidRequestError): """A nonexistent column is requested from a ``RowProxy``.""" diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index d092375a69..8516a22dee 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -235,7 +235,7 @@ class SessionTransaction(object): def _assert_is_open(self, error_msg="The transaction is closed"): if self.session is None: - raise sa_exc.InvalidRequestError(error_msg) + raise sa_exc.ResourceClosedError(error_msg) @property def _is_transaction_boundary(self): @@ -427,7 +427,8 @@ class SessionTransaction(object): return self def __exit__(self, type, value, traceback): - self._assert_is_open("Cannot end transaction context. The transaction was closed from within the context") + self._assert_is_open("Cannot end transaction context. The transaction " + "was closed from within the context") if self.session.transaction is None: return if type is None: diff --git a/test/sql/test_query.py b/test/sql/test_query.py index e8f9d118b3..0a496906d6 100644 --- a/test/sql/test_query.py +++ b/test/sql/test_query.py @@ -16,15 +16,19 @@ class QueryTest(TestBase): users = Table('query_users', metadata, Column('user_id', INT, primary_key=True, test_needs_autoincrement=True), Column('user_name', VARCHAR(20)), + test_needs_acid=True ) addresses = Table('query_addresses', metadata, Column('address_id', Integer, primary_key=True, test_needs_autoincrement=True), Column('user_id', Integer, ForeignKey('query_users.user_id')), - Column('address', String(30))) + Column('address', String(30)), + test_needs_acid=True + ) users2 = Table('u2', metadata, Column('user_id', INT, primary_key = True), Column('user_name', VARCHAR(20)), + test_needs_acid=True ) metadata.create_all() @@ -615,6 +619,39 @@ class QueryTest(TestBase): eq_(r[users.c.user_name], 'jack') eq_(r.user_name, 'jack') + def test_graceful_fetch_on_non_rows(self): + """test that calling fetchone() etc. on a result that doesn't + return rows fails gracefully. + + """ + + # these proxies don't work with no cursor.description present. + # so they don't apply to this test at the moment. + # base.FullyBufferedResultProxy, + # base.BufferedRowResultProxy, + # base.BufferedColumnResultProxy + + conn = testing.db.connect() + for meth in ('fetchone', 'fetchall', 'first', 'scalar', 'fetchmany'): + trans = conn.begin() + result = conn.execute(users.insert(), user_id=1) + assert_raises_message( + exc.ResourceClosedError, + "This result object does not return rows. " + "It has been closed automatically.", + getattr(result, meth), + ) + trans.rollback() + + def test_fetchone_til_end(self): + result = testing.db.execute("select * from query_users") + eq_(result.fetchone(), None) + assert_raises_message( + exc.ResourceClosedError, + "This result object is closed.", + result.fetchone + ) + def test_result_case_sensitivity(self): """test name normalization for result sets."""