- order by secondary works

- self referential is killing it, however

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 259e0f10c13c154f8f717b0e849f1036260a839a..f507bfbe5327a92c21248ebc20e7b606f512cc00 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -711,25 +711,45 @@ class SubqueryLoader(AbstractRelationshipLoader):
             q._order_by = None
         
         q._attributes[('subquery_path', None)] = subq_path
-        
+
         # now select from it as a subquery.
         q = q.from_self(self.mapper, *local_attr)
-        
+
         # and join to the related thing we want
         # to load.
         for mapper, key in [(subq_path[i], subq_path[i+1]) 
-                                for i in xrange(0, len(subq_path), 2)]:
+                            for i in xrange(0, len(subq_path), 2)]:
             prop = mapper.get_property(key)
             q = q.join(prop.class_attribute)
+            
+        #join_on = [(subq_path[i], subq_path[i+1]) 
+        #        for i in xrange(0, len(subq_path), 2)]
+        #for i, (mapper, key) in enumerate(join_on):
+        #    aliased = i != len(join_on) - 1
+        #    prop = mapper.get_property(key)
+        #    q = q.join(prop.class_attribute, aliased=aliased)
 
         q = q.order_by(*local_attr)
         
         # propagate loader options etc. to the new query
         q = q._with_current_path(subq_path)
         q = q._conditional_options(*orig_query._with_options)
-        
+
         if self.parent_property.order_by:
-            q = q.order_by(*self.parent_property.order_by)
+            # if there's an ORDER BY, alias it the same
+            # way joinedloader does, but we have to pull out 
+            # the "eagerjoin" from the query.
+            # this really only picks up the "secondary" table
+            # right now.
+            eagerjoin = q._from_obj[0]
+            eager_order_by = \
+                            eagerjoin._target_adapter.\
+                                copy_and_process(
+                                    util.to_list(
+                                        self.parent_property.order_by
+                                    )
+                                )
+            q = q.order_by(*eager_order_by)
         
         # this key is for the row_processor to pick up
         # within this same loader.

diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index e06aa6ff1d95f15f65c03de8e1dde4248c072fce..81537a8d9868b699f767203a454308c1ab8b0f8a 100644 (file)
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -861,10 +861,12 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
 
     @testing.resolve_artifact_names
     def test_aliasing(self):
-        """test that eager loading uses aliases to insulate the eager load from regular criterion against those tables."""
+        """test that eager loading uses aliases to insulate the eager 
+        load from regular criterion against those tables."""
 
         mapper(User, users, properties = dict(
-            addresses = relationship(mapper(Address, addresses), lazy=False, order_by=addresses.c.id)
+            addresses = relationship(mapper(Address, addresses), 
+                                    lazy=False, order_by=addresses.c.id)
         ))
         q = create_session().query(User)
         l = q.filter(addresses.c.email_address == 'ed@lala.com').filter(
@@ -1096,8 +1098,10 @@ class OrderBySecondaryTest(_base.MappedTest):
         mapper(B, b)
 
         sess = create_session()
-        eq_(sess.query(A).all(), [A(data='a1', bs=[B(data='b3'), B(data='b1'), B(data='b2')]),
-                                  A(bs=[B(data='b4'), B(data='b3'), B(data='b2')])])
+        eq_(sess.query(A).all(), [
+                        A(data='a1', bs=[B(data='b3'), B(data='b1'), B(data='b2')]),
+                        A(bs=[B(data='b4'), B(data='b3'), B(data='b2')])
+        ])
 
 
 class SelfReferentialEagerTest(_base.MappedTest):
@@ -1116,7 +1120,9 @@ class SelfReferentialEagerTest(_base.MappedTest):
                 self.children.append(node)
 
         mapper(Node, nodes, properties={
-            'children':relationship(Node, lazy=False, join_depth=3, order_by=nodes.c.id)
+            'children':relationship(Node, 
+                                        lazy=False, 
+                                        join_depth=3, order_by=nodes.c.id)
         })
         sess = create_session()
         n1 = Node(data='n1')
@@ -1164,7 +1170,8 @@ class SelfReferentialEagerTest(_base.MappedTest):
                 self.children.append(node)
 
         mapper(Node, nodes, properties={
-            'children':relationship(Node, lazy=False, join_depth=1, order_by=nodes.c.id)
+            'children':relationship(Node, lazy=False, join_depth=1,
+                                    order_by=nodes.c.id)
         })
         sess = create_session()
         n1 = Node(data='n1')
@@ -1204,7 +1211,8 @@ class SelfReferentialEagerTest(_base.MappedTest):
                 self.children.append(node)
 
         mapper(Node, nodes, properties={
-            'children':relationship(Node, lazy=False, join_depth=3, order_by=nodes.c.id),
+            'children':relationship(Node, lazy=False, join_depth=3,
+                                    order_by=nodes.c.id),
             'data':deferred(nodes.c.data)
         })
         sess = create_session()
@@ -1233,7 +1241,8 @@ class SelfReferentialEagerTest(_base.MappedTest):
 
         def go():
             eq_(Node(data='n1', children=[Node(data='n11'), Node(data='n12')]),
-                sess.query(Node).options(undefer('data'), undefer('children.data')).first())
+                sess.query(Node).options(undefer('data'),
+                                            undefer('children.data')).first())
         self.assert_sql_count(testing.db, go, 1)
 
 
@@ -1258,7 +1267,8 @@ class SelfReferentialEagerTest(_base.MappedTest):
         sess.flush()
         sess.expunge_all()
         def go():
-            d = sess.query(Node).filter_by(data='n1').options(eagerload('children.children')).first()
+            d = sess.query(Node).filter_by(data='n1').\
+                        options(eagerload('children.children')).first()
             eq_(Node(data='n1', children=[
                 Node(data='n11'),
                 Node(data='n12', children=[
@@ -1271,7 +1281,8 @@ class SelfReferentialEagerTest(_base.MappedTest):
         self.assert_sql_count(testing.db, go, 2)
 
         def go():
-            d = sess.query(Node).filter_by(data='n1').options(eagerload('children.children')).first()
+            d = sess.query(Node).filter_by(data='n1').\
+                        options(eagerload('children.children')).first()
 
         # test that the query isn't wrapping the initial query for eager loading.
         self.assert_sql_execution(testing.db, go, 
@@ -1320,14 +1331,14 @@ class MixedSelfReferentialEagerTest(_base.MappedTest):
     @classmethod
     def define_tables(cls, metadata):
         Table('a_table', metadata,
-                       Column('id', Integer, primary_key=True, test_needs_autoincrement=True)
-                       )
+               Column('id', Integer, primary_key=True, test_needs_autoincrement=True)
+               )
 
         Table('b_table', metadata,
-                       Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
-                       Column('parent_b1_id', Integer, ForeignKey('b_table.id')),
-                       Column('parent_a_id', Integer, ForeignKey('a_table.id')),
-                       Column('parent_b2_id', Integer, ForeignKey('b_table.id')))
+               Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
+               Column('parent_b1_id', Integer, ForeignKey('b_table.id')),
+               Column('parent_a_id', Integer, ForeignKey('a_table.id')),
+               Column('parent_b2_id', Integer, ForeignKey('b_table.id')))
 
 
     @classmethod
@@ -1379,7 +1390,11 @@ class MixedSelfReferentialEagerTest(_base.MappedTest):
         session = create_session()
         def go():
             eq_(
-                session.query(B).options(eagerload('parent_b1'),eagerload('parent_b2'),eagerload('parent_z')).
+                session.query(B).\
+                    options(
+                                eagerload('parent_b1'),
+                                eagerload('parent_b2'),
+                                eagerload('parent_z')).
                             filter(B.id.in_([2, 8, 11])).order_by(B.id).all(),
                 [
                     B(id=2, parent_z=A(id=1), parent_b1=B(id=1), parent_b2=None),

diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py
index 6385bbb5325e1d375f1cba809966ef21bfaf8e0e..e1372fbfe67e7df55bd7d1a6ce0dc94284787b20 100644 (file)
--- a/test/orm/test_subquery_relations.py
+++ b/test/orm/test_subquery_relations.py
@@ -1,8 +1,11 @@
 from sqlalchemy.test.testing import eq_, is_, is_not_
 from sqlalchemy.test import testing
+from sqlalchemy.test.schema import Table, Column
+from sqlalchemy import Integer, String, ForeignKey
 from sqlalchemy.orm import backref, subqueryload, subqueryload_all, \
                 mapper, relationship, clear_mappers,\
-                create_session, lazyload, aliased, eagerload
+                create_session, lazyload, aliased, eagerload,\
+                deferred
 from sqlalchemy.test.testing import eq_, assert_raises
 from sqlalchemy.test.assertsql import CompiledSQL
 from test.orm import _base, _fixtures
@@ -512,6 +515,263 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
         assert_raises(sa.exc.SAWarning,
                 s.query(User).options(subqueryload(User.order)).all)
 
+class OrderBySecondaryTest(_base.MappedTest):
+    @classmethod
+    def define_tables(cls, metadata):
+        Table('m2m', metadata,
+              Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
+              Column('aid', Integer, ForeignKey('a.id')),
+              Column('bid', Integer, ForeignKey('b.id')))
+
+        Table('a', metadata,
+              Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
+              Column('data', String(50)))
+        Table('b', metadata,
+              Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
+              Column('data', String(50)))
+
+    @classmethod
+    def fixtures(cls):
+        return dict(
+            a=(('id', 'data'),
+               (1, 'a1'),
+               (2, 'a2')),
+
+            b=(('id', 'data'),
+               (1, 'b1'),
+               (2, 'b2'),
+               (3, 'b3'),
+               (4, 'b4')),
+
+            m2m=(('id', 'aid', 'bid'),
+                 (2, 1, 1),
+                 (4, 2, 4),
+                 (1, 1, 3),
+                 (6, 2, 2),
+                 (3, 1, 2),
+                 (5, 2, 3)))
+
+    @testing.resolve_artifact_names
+    def test_ordering(self):
+        class A(_base.ComparableEntity):pass
+        class B(_base.ComparableEntity):pass
+
+        mapper(A, a, properties={
+            'bs':relationship(B, secondary=m2m, lazy='subquery', order_by=m2m.c.id)
+        })
+        mapper(B, b)
+
+        sess = create_session()
+        def go():
+            eq_(sess.query(A).all(), [
+                        A(data='a1', bs=[B(data='b3'), B(data='b1'), B(data='b2')]),
+                        A(bs=[B(data='b4'), B(data='b3'), B(data='b2')])
+            ])
+        self.assert_sql_count(testing.db, go, 2)
+
+class SelfReferentialEagerTest(_base.MappedTest):
+    @classmethod
+    def define_tables(cls, metadata):
+        Table('nodes', metadata,
+              Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
+            Column('parent_id', Integer, ForeignKey('nodes.id')),
+            Column('data', String(30)))
+
+    @testing.fails_on('maxdb', 'FIXME: unknown')
+    @testing.resolve_artifact_names
+    def _test_basic(self):
+        class Node(_base.ComparableEntity):
+            def append(self, node):
+                self.children.append(node)
+
+        mapper(Node, nodes, properties={
+            'children':relationship(Node, 
+                                        lazy='subquery', 
+                                        join_depth=3, order_by=nodes.c.id)
+        })
+        sess = create_session()
+        n1 = Node(data='n1')
+        n1.append(Node(data='n11'))
+        n1.append(Node(data='n12'))
+        n1.append(Node(data='n13'))
+#        n1.children[1].append(Node(data='n121'))
+#        n1.children[1].append(Node(data='n122'))
+#        n1.children[1].append(Node(data='n123'))
+        n2 = Node(data='n2')
+        n2.append(Node(data='n21'))
+#        n2.children[0].append(Node(data='n211'))
+#        n2.children[0].append(Node(data='n212'))
+        
+        sess.add(n1)
+        sess.add(n2)
+        sess.flush()
+        sess.expunge_all()
+        def go():
+            d = sess.query(Node).filter(Node.data.in_(['n1', 'n2'])).\
+                            order_by(Node.data).all()
+            eq_([Node(data='n1', children=[
+                    Node(data='n11'),
+                    Node(data='n12', children=[
+#                        Node(data='n121'),
+#                        Node(data='n122'),
+#                        Node(data='n123')
+                    ]),
+                    Node(data='n13')
+                ]),
+                Node(data='n2', children=[
+                    Node(data='n21', children=[
+#                        Node(data='n211'),
+#                        Node(data='n212'),
+                    ])
+                ])
+            ], d)
+        self.assert_sql_count(testing.db, go, 1)
+
+
+
+    @testing.resolve_artifact_names
+    def _test_lazy_fallback_doesnt_affect_eager(self):
+        class Node(_base.ComparableEntity):
+            def append(self, node):
+                self.children.append(node)
+
+        mapper(Node, nodes, properties={
+            'children':relationship(Node, lazy='subquery', join_depth=1,
+                                    order_by=nodes.c.id)
+        })
+        sess = create_session()
+        n1 = Node(data='n1')
+        n1.append(Node(data='n11'))
+        n1.append(Node(data='n12'))
+        n1.append(Node(data='n13'))
+        n1.children[1].append(Node(data='n121'))
+        n1.children[1].append(Node(data='n122'))
+        n1.children[1].append(Node(data='n123'))
+        sess.add(n1)
+        sess.flush()
+        sess.expunge_all()
+
+        def go():
+            allnodes = sess.query(Node).order_by(Node.data).all()
+            n12 = allnodes[2]
+            eq_(n12.data, 'n12')
+            eq_([
+                Node(data='n121'),
+                Node(data='n122'),
+                Node(data='n123')
+            ], list(n12.children))
+        self.assert_sql_count(testing.db, go, 1)
+
+    @testing.resolve_artifact_names
+    def _test_with_deferred(self):
+        class Node(_base.ComparableEntity):
+            def append(self, node):
+                self.children.append(node)
+
+        mapper(Node, nodes, properties={
+            'children':relationship(Node, lazy='subquery', join_depth=3,
+                                    order_by=nodes.c.id),
+            'data':deferred(nodes.c.data)
+        })
+        sess = create_session()
+        n1 = Node(data='n1')
+        n1.append(Node(data='n11'))
+        n1.append(Node(data='n12'))
+        sess.add(n1)
+        sess.flush()
+        sess.expunge_all()
+
+        def go():
+            eq_( 
+                Node(data='n1', children=[Node(data='n11'), Node(data='n12')]),
+                sess.query(Node).order_by(Node.id).first(),
+                )
+        self.assert_sql_count(testing.db, go, 4)
+
+        sess.expunge_all()
+
+        def go():
+            eq_(Node(data='n1', children=[Node(data='n11'), Node(data='n12')]),
+                sess.query(Node).options(undefer('data')).order_by(Node.id).first())
+        self.assert_sql_count(testing.db, go, 3)
+
+        sess.expunge_all()
+
+        def go():
+            eq_(Node(data='n1', children=[Node(data='n11'), Node(data='n12')]),
+                sess.query(Node).options(undefer('data'),
+                                            undefer('children.data')).first())
+        self.assert_sql_count(testing.db, go, 1)
+
+
+    @testing.resolve_artifact_names
+    def _test_options(self):
+        class Node(_base.ComparableEntity):
+            def append(self, node):
+                self.children.append(node)
+
+        mapper(Node, nodes, properties={
+            'children':relationship(Node, lazy=True, order_by=nodes.c.id)
+        }, order_by=nodes.c.id)
+        sess = create_session()
+        n1 = Node(data='n1')
+        n1.append(Node(data='n11'))
+        n1.append(Node(data='n12'))
+        n1.append(Node(data='n13'))
+        n1.children[1].append(Node(data='n121'))
+        n1.children[1].append(Node(data='n122'))
+        n1.children[1].append(Node(data='n123'))
+        sess.add(n1)
+        sess.flush()
+        sess.expunge_all()
+        def go():
+            d = sess.query(Node).filter_by(data='n1').\
+                        options(eagerload('children.children')).first()
+            eq_(Node(data='n1', children=[
+                Node(data='n11'),
+                Node(data='n12', children=[
+                    Node(data='n121'),
+                    Node(data='n122'),
+                    Node(data='n123')
+                ]),
+                Node(data='n13')
+            ]), d)
+        self.assert_sql_count(testing.db, go, 2)
+
+    @testing.fails_on('maxdb', 'FIXME: unknown')
+    @testing.resolve_artifact_names
+    def _test_no_depth(self):
+        class Node(_base.ComparableEntity):
+            def append(self, node):
+                self.children.append(node)
+
+        mapper(Node, nodes, properties={
+            'children':relationship(Node, lazy='subquery')
+        })
+        sess = create_session()
+        n1 = Node(data='n1')
+        n1.append(Node(data='n11'))
+        n1.append(Node(data='n12'))
+        n1.append(Node(data='n13'))
+        n1.children[1].append(Node(data='n121'))
+        n1.children[1].append(Node(data='n122'))
+        n1.children[1].append(Node(data='n123'))
+        sess.add(n1)
+        sess.flush()
+        sess.expunge_all()
+        def go():
+            d = sess.query(Node).filter_by(data='n1').first()
+            eq_(Node(data='n1', children=[
+                Node(data='n11'),
+                Node(data='n12', children=[
+                    Node(data='n121'),
+                    Node(data='n122'),
+                    Node(data='n123')
+                ]),
+                Node(data='n13')
+            ]), d)
+        self.assert_sql_count(testing.db, go, 3)
+
     # TODO: all the tests in test_eager_relations
     
     # TODO: ensure state stuff works out OK, existing objects/collections