From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Wed, 17 Oct 2007 16:49:15 +0000 (+0000)
Subject: - removed __len__ from "dynamic" collection as it would require issuing
X-Git-Tag: rel_0_4_0~3
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=cb9321116fcdf350461410a5ecf8b892cfa210a5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- removed __len__ from "dynamic" collection as it would require issuing
  a SQL "count()" operation, thus forcing all list evaluations to issue
  redundant SQL [ticket:818]
---

diff --git a/CHANGES b/CHANGES
index a09d22be60..85ffc40def 100644
--- a/CHANGES
+++ b/CHANGES
@@ -17,6 +17,10 @@ CHANGES
   collection doesn't contain the item, supports noload collections
   [ticket:813]
 
+- removed __len__ from "dynamic" collection as it would require issuing
+  a SQL "count()" operation, thus forcing all list evaluations to issue
+  redundant SQL [ticket:818]
+  
 - inline optimizations added to locate_dirty() which can greatly speed up
   repeated calls to flush(), as occurs with autoflush=True [ticket:816]
   
diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py
index 1b91bd977c..993756edeb 100644
--- a/lib/sqlalchemy/orm/dynamic.py
+++ b/lib/sqlalchemy/orm/dynamic.py
@@ -71,13 +71,6 @@ class AppenderQuery(Query):
         else:
             return sess
             
-    def __len__(self):
-        sess = self.__session()
-        if sess is None:
-            return len(self.attr.get_history(self.state)._added_items)
-        else:
-            return self._clone(sess).count()
-        
     def __iter__(self):
         sess = self.__session()
         if sess is None:
diff --git a/test/orm/dynamic.py b/test/orm/dynamic.py
index 1c294f4fcf..7d0d748e3d 100644
--- a/test/orm/dynamic.py
+++ b/test/orm/dynamic.py
@@ -26,6 +26,19 @@ class DynamicTest(QueryTest):
         assert [User(id=7, addresses=[Address(id=1, email_address='jack@bean.com')])] == q.filter(User.id==7).all()
         assert fixtures.user_address_result == q.all()
 
+    def test_no_count(self):
+        mapper(User, users, properties={
+            'addresses':dynamic_loader(mapper(Address, addresses))
+        })
+        sess = create_session()
+        q = sess.query(User)
+
+        # dynamic collection cannot implement __len__() (at least one that returns a live database
+        # result), else additional count() queries are issued when evaluating in a list context
+        def go():
+            assert [User(id=7, addresses=[Address(id=1, email_address='jack@bean.com')])] == q.filter(User.id==7).all()
+        self.assert_sql_count(testbase.db, go, 2)
+        
 class FlushTest(FixtureTest):
     def test_basic(self):
         class Fixture(Base):
diff --git a/test/testlib/fixtures.py b/test/testlib/fixtures.py
index ada254c375..6baf16bf07 100644
--- a/test/testlib/fixtures.py
+++ b/test/testlib/fixtures.py
@@ -42,10 +42,13 @@ class Base(object):
                     except AttributeError:
                         #print "Other class does not have attribute named '%s'" % attr
                         return False
-                    if len(value) != len(getattr(other, attr)):
+                    if not hasattr(value, '__len__'):
+                        value = list(iter(value))
+                        otherattr = list(iter(otherattr))
+                    if len(value) != len(otherattr):
                         #print "Length of collection '%s' does not match that of other" % attr
                         return False
-                    for (us, them) in zip(value, getattr(other, attr)):
+                    for (us, them) in zip(value, otherattr):
                         if us != them:
                             #print "1. Attribute named '%s' does not match other" % attr
                             return False