- fixed critical issue when, after options(eagerload()) is used,

the mapper would then always apply query "wrapping" behavior
for all subsequent LIMIT/OFFSET/DISTINCT queries, even if no
eager loading was applied on those subsequent queries.

5 files changed, 31 insertions, 3 deletions

diff --git a/CHANGES b/CHANGES
index 5b65adfe8..c560f4fe7 100644
--- a/CHANGES
+++ b/CHANGES
@@ -58,6 +58,10 @@
     - slight tweak to raw execute() change to also support tuples
       for positional parameters, not just lists [ticket:523]
 - orm:
+    - fixed critical issue when, after options(eagerload()) is used,
+      the mapper would then always apply query "wrapping" behavior
+      for all subsequent LIMIT/OFFSET/DISTINCT queries, even if no
+      eager loading was applied on those subsequent queries.
     - added query.with_parent(someinstance) method.  searches for
       target instance using lazy join criterion from parent instance.
       takes optional string "property" to isolate the desired relation.
diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index 5c6a92325..358e1297f 100644
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -149,8 +149,8 @@ class StrategizedProperty(MapperProperty):
         except KeyError:
             # cache the located strategy per class for faster re-lookup
             strategy = cls(self)
-            strategy.init()
             strategy.is_default = False
+            strategy.init()
             self._all_strategies[cls] = strategy
             return strategy
 
@@ -282,7 +282,7 @@ class LoaderStrategy(object):
     def init(self):
         self.parent = self.parent_property.parent
         self.key = self.parent_property.key
-
+    
     def init_class_attribute(self):
         pass
 
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index e9d992d18..ee80b3477 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -317,7 +317,8 @@ class EagerLoader(AbstractRelationLoader):
                 "to child class '%s': Cant use eager loading on a self "
                 "referential relationship." %
                 (self.key, repr(self.parent.class_), repr(self.mapper.class_)))
-        self.parent._eager_loaders.add(self.parent_property)
+        if self.is_default:
+            self.parent._eager_loaders.add(self.parent_property)
 
         self.clauses = {}
         self.clauses_by_lead_mapper = {}
diff --git a/test/orm/mapper.py b/test/orm/mapper.py
index b2118b647..d1c4cf546 100644
--- a/test/orm/mapper.py
+++ b/test/orm/mapper.py
@@ -541,6 +541,15 @@ class MapperTest(MapperSuperTest):
             assert len(u.addresses) == 3
         self.assert_sql_count(db, go, 0)
 
+        sess.clear()
+        
+        # test that eager loading doesnt modify parent mapper
+        def go():
+            u = sess.query(User).get_by(user_id=8)
+            assert u.user_id == 8
+            assert len(u.addresses) == 3
+        assert "tbl_row_count" not in self.capture_sql(db, go)
+        
     def testlazyoptionswithlimit(self):
         sess = create_session()
         mapper(User, users, properties = dict(
diff --git a/test/testbase.py b/test/testbase.py
index aae455673..1bcc5c142 100644
--- a/test/testbase.py
+++ b/test/testbase.py
@@ -224,6 +224,17 @@ class AssertMixin(PersistTest):
         finally:
             self.assert_(testdata.sql_count == count, "desired statement count %d does not match %d" % (count, testdata.sql_count))
 
+    def capture_sql(self, db, callable_):
+        global testdata
+        testdata = TestData(db)
+        buffer = StringIO.StringIO()
+        testdata.buffer = buffer
+        try:
+            callable_()
+            return buffer.getvalue()
+        finally:
+            testdata.buffer = None
+            
 class ORMTest(AssertMixin):
     keep_mappers = False
     keep_data = False
@@ -251,6 +262,7 @@ class TestData(object):
         self.logger = engine.logger
         self.set_assert_list(None, None)
         self.sql_count = 0
+        self.buffer = None
         
     def set_assert_list(self, unittest, list):
         self.unittest = unittest
@@ -270,6 +282,8 @@ class ExecutionContextWrapper(object):
         ctx = self.ctx
         statement = unicode(ctx.compiled)
         statement = re.sub(r'\n', '', ctx.statement)
+        if testdata.buffer is not None:
+            testdata.buffer.write(statement + "\n")
 
         if testdata.assert_list is not None:
             item = testdata.assert_list[-1]