- improved support for eagerloading of properties off of mappers that are mapped

to select() statements; i.e. eagerloader is better at locating the correct
selectable with which to attach its LEFT OUTER JOIN.
- some fixes to new tests in inheritance5 to work with postgres

diff --git a/CHANGES b/CHANGES
index 74c818c150f65c58b0a90ebd2207b55c8696d0a6..368956259d701d5e02a1e1b1f7363849da8429a1 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -39,6 +39,9 @@
       local table.
     - restored logging of "lazy loading clause" under sa.orm.strategies logger,
       got removed in 0.3.7
+    - improved support for eagerloading of properties off of mappers that are mapped
+      to select() statements; i.e. eagerloader is better at locating the correct
+      selectable with which to attach its LEFT OUTER JOIN.
 - mysql
     - support for column-level CHARACTER SET and COLLATE declarations,
       as well as ASCII, UNICODE, NATIONAL and BINARY shorthand.

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 6bdaa1d16137d0d0cd081fa2ba8938a08d55887d..727d949bc60f573acf8da4f77a1f7aca4251f0df 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -470,22 +470,23 @@ class EagerLoader(AbstractRelationLoader):
         
         if hasattr(statement, '_outerjoin'):
             towrap = statement._outerjoin
-        elif isinstance(localparent.mapped_table, schema.Table):
-            # if the mapper is against a plain Table, look in the from_obj of the select statement
-            # to join against whats already there.
-            for (fromclause, finder) in [(x, sql_util.TableFinder(x)) for x in statement.froms]:
-                # dont join against an Alias'ed Select.  we are really looking either for the 
-                # table itself or a Join that contains the table.  this logic still might need
-                # adjustments for scenarios not thought of yet.
-                if not isinstance(fromclause, sql.Alias) and localparent.mapped_table in finder:
+        elif isinstance(localparent.mapped_table, sql.Join):
+            towrap = localparent.mapped_table
+        else:
+            # look for the mapper's selectable expressed within the current "from" criterion.
+            # this will locate the selectable inside of any containers it may be a part of (such
+            # as a join).  if its inside of a join, we want to outer join on that join, not the 
+            # selectable.
+            for fromclause in statement.froms:
+                if fromclause is localparent.mapped_table:
                     towrap = fromclause
                     break
+                elif isinstance(fromclause, sql.Join):
+                    if localparent.mapped_table in sql_util.TableFinder(fromclause, include_aliases=True):
+                        towrap = fromclause
+                        break
             else:
-                raise exceptions.InvalidRequestError("EagerLoader cannot locate a clause with which to outer join to, in query '%s' %s" % (str(statement), self.localparent.mapped_table))
-        else:
-            # if the mapper is against a select statement or something, we cant handle that at the
-            # same time as a custom FROM clause right now.
-            towrap = localparent.mapped_table
+                raise exceptions.InvalidRequestError("EagerLoader cannot locate a clause with which to outer join to, in query '%s' %s" % (str(statement), localparent.mapped_table))
         
         try:
             clauses = self.clauses[parentclauses]

diff --git a/lib/sqlalchemy/sql_util.py b/lib/sqlalchemy/sql_util.py
index 1f5ac168118d59c04f803968ae4011d2958b4a8f..debf1da4f9c5b5ca37216b2d9e360e3be74ec39b 100644 (file)
--- a/lib/sqlalchemy/sql_util.py
+++ b/lib/sqlalchemy/sql_util.py
@@ -65,14 +65,19 @@ class TableCollection(object):
 
 
 class TableFinder(TableCollection, sql.NoColumnVisitor):
-    """Given a ``Clause``, locate all the ``Tables`` within it into a list."""
+    """locate all Tables within a clause."""
 
-    def __init__(self, table, check_columns=False):
+    def __init__(self, table, check_columns=False, include_aliases=False):
         TableCollection.__init__(self)
         self.check_columns = check_columns
+        self.include_aliases = include_aliases
         if table is not None:
             self.traverse(table)
 
+    def visit_alias(self, alias):
+        if self.include_aliases:
+            self.tables.append(alias)
+            
     def visit_table(self, table):
         self.tables.append(table)
 

diff --git a/test/orm/inheritance5.py b/test/orm/inheritance5.py
index 1c42146356493bc72962de6daaa352b765c25685..56a83f5757a305caa19ca3a4ec7d8999a6daec63 100644 (file)
--- a/test/orm/inheritance5.py
+++ b/test/orm/inheritance5.py
@@ -706,7 +706,7 @@ class GenerativeTest(testbase.AssertMixin):
             # into the WHERE criterion, using a correlated select. ticket #577 tracks 
             # that Query's adaptation of the WHERE clause does not dig into the 
             # mapped selectable itself, which permanently breaks the mapped selectable.
-            r = session.query(Person).filter(Car.c.owner == select([Car.c.owner], Car.c.owner==employee_join.c.person_id))
+            r = session.query(Person).filter(exists([Car.c.owner], Car.c.owner==employee_join.c.person_id))
             assert str(list(r)) == "[Engineer E4, field X, status Status dead]"
         
 class MultiLevelTest(testbase.ORMTest):
@@ -878,7 +878,7 @@ class CustomPKTest(testbase.ORMTest):
         
         # query using get(), using only one value.  this requires the select_table mapper
         # has the same single-col primary key.
-        assert sess.query(T1).get(ot1.id).id is ot1.id
+        assert sess.query(T1).get(ot1.id).id == ot1.id
         
         ot1 = sess.query(T1).get(ot1.id)
         ot1.data = 'hi'

diff --git a/test/orm/mapper.py b/test/orm/mapper.py
index 889a7c925972c896ae04a8224cf2e87aac9a111c..921b5f9b06776d8036d80dd16c943dbf42e24b45 100644 (file)
--- a/test/orm/mapper.py
+++ b/test/orm/mapper.py
@@ -1325,6 +1325,27 @@ class EagerTest(MapperSuperTest):
         
         l = m.instances(s.execute(emailad = 'jack@bean.com'), session)
         self.echo(repr(l))
+    
+    def testonselect(self):
+        """test eager loading of a mapper which is against a select"""
+        
+        s = select([orders], orders.c.isopen==1).alias('openorders')
+        mapper(Order, s, properties={
+            'user':relation(User, lazy=False)
+        })
+        mapper(User, users)
+        
+        q = create_session().query(Order)
+        self.assert_result(q.list(), Order,
+            {'order_id':3, 'user' : (User, {'user_id':7})},
+            {'order_id':4, 'user' : (User, {'user_id':9})},
+        )
+
+        q = q.select_from(s.outerjoin(orderitems)).filter(orderitems.c.item_name != 'item 2')
+        self.assert_result(q.list(), Order,
+            {'order_id':3, 'user' : (User, {'user_id':7})},
+        )
+        
         
     def testmulti(self):
         """tests eager loading with two relations simultaneously"""