From: Mike Bayer Date: Tue, 13 Apr 2010 00:20:50 +0000 (-0400) Subject: further testing reveals that cursor.rowcount is only called with update/delete and... X-Git-Tag: rel_0_6_0~23^2^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=731ad9b0b956fb3b9d318576661ede51b3d4f596;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git further testing reveals that cursor.rowcount is only called with update/delete and DDL, and also that FB's cursor.rowcount is a little expensive, but not dramatically. added a test to ensure cursor.rowcount is only called on update/delete. the current default for firebird enable_rowcount is now True, leaving all the options to turn it off etc.. --- diff --git a/CHANGES b/CHANGES index b8031869db..5d79075b58 100644 --- a/CHANGES +++ b/CHANGES @@ -136,13 +136,15 @@ CHANGES apply any kind of conversions. - firebird - - The functionality of result.rowcount is now disabled - by default, and can be re-enabled using the 'enable_rowcount' - flag with create_engine(), as well as the 'enable_rowcount' - execution context flag on a per-execute basis. This because - cursor.rowcount requires cursor access (can't be evaluated - lazily since the result auto-closes) and also incurs an - expensive round-trip. + - The functionality of result.rowcount can be disabled on a + per-engine basis by setting 'enable_rowcount=False' + on create_engine(). Normally, cursor.rowcount is called + after any UPDATE or DELETE statement unconditionally, + because the cursor is then closed and Firebird requires + an open cursor in order to get a rowcount. This + call is slightly expensive however so it can be disabled. + To re-enable on a per-execution basis, the + 'enable_rowcount=True' execution option may be used. - examples - Updated attribute_shard.py example to use a more robust diff --git a/lib/sqlalchemy/dialects/firebird/kinterbasdb.py b/lib/sqlalchemy/dialects/firebird/kinterbasdb.py index 890ba83fe6..84963ba8b7 100644 --- a/lib/sqlalchemy/dialects/firebird/kinterbasdb.py +++ b/lib/sqlalchemy/dialects/firebird/kinterbasdb.py @@ -19,15 +19,19 @@ Kinterbasedb backend specific keyword arguments are: * concurrency_level - set the backend policy with regards to threading issues: by default SQLAlchemy uses policy 1 (see details__). -* enable_rowcount - False by default, this enables the usage of "cursor.rowcount" with the - Kinterbasdb dialect. When disabled, SQLAlchemy's ResultProxy will - return -1 for result.rowcount. The rationale here is that Kinterbasdb - requires a second round trip to the database when .rowcount is called - - since SQLA's resultproxy automatically closes the cursor after a - non-result-returning statement, rowcount must be called, if at all, - before the result object is returned. The behavior can also be - controlled on a per-execution basis using the `enable_rowcount` - option with :meth:`execution_options()`:: +* enable_rowcount - True by default, setting this to False disables + the usage of "cursor.rowcount" with the + Kinterbasdb dialect, which SQLAlchemy ordinarily calls upon automatically + after any UPDATE or DELETE statement. When disabled, SQLAlchemy's + ResultProxy will return -1 for result.rowcount. The rationale here is + that Kinterbasdb requires a second round trip to the database when + .rowcount is called - since SQLA's resultproxy automatically closes + the cursor after a non-result-returning statement, rowcount must be + called, if at all, before the result object is returned. Additionally, + cursor.rowcount may not return correct results with older versions + of Firebird, and setting this flag to False will also cause the SQLAlchemy ORM + to ignore its usage. The behavior can also be controlled on a per-execution + basis using the `enable_rowcount` option with :meth:`execution_options()`:: conn = engine.connect().execution_options(enable_rowcount=True) r = conn.execute(stmt) @@ -77,7 +81,7 @@ class FBDialect_kinterbasdb(FBDialect): ) - def __init__(self, type_conv=200, concurrency_level=1, enable_rowcount=False, **kwargs): + def __init__(self, type_conv=200, concurrency_level=1, enable_rowcount=True, **kwargs): super(FBDialect_kinterbasdb, self).__init__(**kwargs) self.enable_rowcount = enable_rowcount self.type_conv = type_conv diff --git a/test/dialect/test_firebird.py b/test/dialect/test_firebird.py index a96a528be2..9bdce8ff71 100644 --- a/test/dialect/test_firebird.py +++ b/test/dialect/test_firebird.py @@ -1,4 +1,4 @@ -from sqlalchemy.test.testing import eq_ +from sqlalchemy.test.testing import eq_, assert_raises from sqlalchemy import * from sqlalchemy.databases import firebird from sqlalchemy.exc import ProgrammingError @@ -304,7 +304,6 @@ class MiscTest(TestBase): r = t.delete().execution_options(enable_rowcount=False).execute() eq_(r.rowcount, -1) - engine = engines.testing_engine(options={'enable_rowcount':False}) assert not engine.dialect.supports_sane_rowcount metadata.bind = engine @@ -315,7 +314,7 @@ class MiscTest(TestBase): eq_(r.rowcount, -1) r = t.delete().execution_options(enable_rowcount=True).execute() eq_(r.rowcount, 1) - + def test_percents_in_text(self): for expr, result in ( (text("select '%' from rdb$database"), '%'), diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py index de1de225f5..3f3f0e2d56 100644 --- a/test/engine/test_execute.py +++ b/test/engine/test_execute.py @@ -1,9 +1,8 @@ -from sqlalchemy.test.testing import eq_ +from sqlalchemy.test.testing import eq_, assert_raises import re from sqlalchemy.interfaces import ConnectionProxy from sqlalchemy import MetaData, Integer, String, INT, VARCHAR, func, bindparam, select -from sqlalchemy.test.schema import Table -from sqlalchemy.test.schema import Column +from sqlalchemy.test.schema import Table, Column import sqlalchemy as tsa from sqlalchemy.test import TestBase, testing, engines import logging @@ -181,7 +180,7 @@ class LogTest(TestBase): "0x...%s" % hex(id(eng.pool))[-4:], ) -class RowInterpTest(TestBase): +class ResultProxyTest(TestBase): def test_nontuple_row(self): """ensure the C version of BaseRowProxy handles duck-type-dependent rows.""" @@ -202,6 +201,45 @@ class RowInterpTest(TestBase): eq_(list(proxy), ['value']) eq_(proxy[0], 'value') eq_(proxy['key'], 'value') + + @testing.provide_metadata + def test_no_rowcount_on_selects_inserts(self): + """assert that rowcount is only called on deletes and updates. + + This because cursor.rowcount can be expensive on some dialects + such as Firebird. + + """ + + engine = engines.testing_engine() + metadata.bind = engine + + t = Table('t1', metadata, + Column('data', String(10)) + ) + metadata.create_all() + + class BreakRowcountMixin(object): + @property + def rowcount(self): + assert False + + execution_ctx_cls = engine.dialect.execution_ctx_cls + engine.dialect.execution_ctx_cls = type("FakeCtx", + (BreakRowcountMixin, + execution_ctx_cls), + {}) + + try: + r = t.insert().execute({'data':'d1'}, {'data':'d2'}, {'data': 'd3'}) + eq_( + t.select().execute().fetchall(), + [('d1', ), ('d2',), ('d3', )] + ) + assert_raises(AssertionError, t.update().execute, {'data':'d4'}) + assert_raises(AssertionError, t.delete().execute) + finally: + engine.dialect.execution_ctx_cls = execution_ctx_cls class ProxyConnectionTest(TestBase):