- _with_parent_criterion generalized into _with_lazy_criterion

- _create_lazy_clause now includes a 'reverse_direction' flag to generate lazy criterion
in from parent->child or vice versa
- changed join_by() in query to use the "reverse" _create_lazy_clause for instance comparisons
so conditions like AND can work [ticket:554]

diff --git a/CHANGES b/CHANGES
index fea91b7f51eed99f9ea77829512a2ac49f3ec6bd..07c65aac85afe4f99bf34c4292a754f2e3272889 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -80,6 +80,10 @@
       takes optional string "property" to isolate the desired relation.
       also adds static Query.query_from_parent(instance, property)
       version. [ticket:541]
+    - improved query.XXX_by(someprop=someinstance) querying to use
+      similar methodology to with_parent, i.e. using the "lazy" clause
+      which prevents adding the remote instance's table to the SQL, 
+      thereby making more complex conditions possible [ticket:554]
     - added generative versions of aggregates, i.e. sum(), avg(), etc.
       to query. used via query.apply_max(), apply_sum(), etc. 
       #552

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index c43b9a94608262e58d42eaf0a24291f90aa90348..edbd53fdcaeab533057ac26723c903994e29db68 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -365,27 +365,25 @@ class Query(object):
         t = sql.text(text)
         return self.execute(t, params=params)
 
-    def _with_parent_criterion(cls, mapper, instance, prop):
+    def _with_lazy_criterion(cls, instance, prop, reverse=False):
         """extract query criterion from a LazyLoader strategy given a Mapper, 
         source persisted/detached instance and PropertyLoader.
         
-        ."""
+        """
+        
         from sqlalchemy.orm import strategies
-        # this could be done very slickly with prop.compare() and join_via(),
-        # but we are using the less generic LazyLoader clause so that we
-        # retain the ability to do a self-referential join (also the lazy 
-        # clause typically requires only the primary table with no JOIN)
-        loader = prop._get_strategy(strategies.LazyLoader)
-        criterion = loader.lazywhere.copy_container()
-        bind_to_col = dict([(loader.lazybinds[col].key, col) for col in loader.lazybinds])
+        (criterion, lazybinds, rev) = strategies.LazyLoader._create_lazy_clause(prop, reverse_direction=reverse)
+        bind_to_col = dict([(lazybinds[col].key, col) for col in lazybinds])
 
         class Visitor(sql.ClauseVisitor):
             def visit_bindparam(self, bindparam):
+                mapper = reverse and prop.mapper or prop.parent
                 bindparam.value = mapper.get_attr_by_column(instance, bind_to_col[bindparam.key])
         Visitor().traverse(criterion)
         return criterion
-    _with_parent_criterion = classmethod(_with_parent_criterion)
+    _with_lazy_criterion = classmethod(_with_lazy_criterion)
     
+        
     def query_from_parent(cls, instance, property, **kwargs):
         """return a newly constructed Query object, with criterion corresponding to 
         a relationship to the given parent instance.
@@ -407,7 +405,7 @@ class Query(object):
         mapper = object_mapper(instance)
         prop = mapper.props[property]
         target = prop.mapper
-        criterion = cls._with_parent_criterion(mapper, instance, prop)
+        criterion = cls._with_lazy_criterion(instance, prop)
         return Query(target, **kwargs).filter(criterion)
     query_from_parent = classmethod(query_from_parent)
         
@@ -436,7 +434,7 @@ class Query(object):
                 raise exceptions.InvalidRequestError("Could not locate a property which relates instances of class '%s' to instances of class '%s'" % (self.mapper.class_.__name__, instance.__class__.__name__))
         else:
             prop = mapper.props[property]
-        return self.filter(Query._with_parent_criterion(mapper, instance, prop))
+        return self.filter(Query._with_lazy_criterion(instance, prop))
 
     def add_entity(self, entity):
         """add a mapped entity to the list of result columns to be returned.
@@ -567,7 +565,8 @@ class Query(object):
         The criterion is constructed in the same way as the
         ``select_by()`` method.
         """
-
+        import properties
+        
         clause = None
         for arg in args:
             if clause is None:
@@ -577,7 +576,10 @@ class Query(object):
 
         for key, value in params.iteritems():
             (keys, prop) = self._locate_prop(key, start=start)
-            c = prop.compare(value) & self.join_via(keys)
+            if isinstance(prop, properties.PropertyLoader):
+                c = self._with_lazy_criterion(value, prop, True) & self.join_via(keys[:-1])
+            else:
+                c = prop.compare(value) & self.join_via(keys)
             if clause is None:
                 clause =  c
             else:

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index ee80b347757585c14fdabb64c638812c0e4129b7..0eb05840f9c26945c03fc2f4d4640ae9426a2755 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -161,7 +161,7 @@ NoLoader.logger = logging.class_logger(NoLoader)
 class LazyLoader(AbstractRelationLoader):
     def init(self):
         super(LazyLoader, self).init()
-        (self.lazywhere, self.lazybinds, self.lazyreverse) = self._create_lazy_clause(self.polymorphic_primaryjoin, self.polymorphic_secondaryjoin, self.remote_side)
+        (self.lazywhere, self.lazybinds, self.lazyreverse) = self._create_lazy_clause(self)
 
         # determine if our "lazywhere" clause is the same as the mapper's
         # get() clause.  then we can just use mapper.get()
@@ -246,12 +246,17 @@ class LazyLoader(AbstractRelationLoader):
                 # to load data into it.
                 sessionlib.attribute_manager.reset_instance_attribute(instance, self.key)
 
-    def _create_lazy_clause(self, primaryjoin, secondaryjoin, remote_side):
+    def _create_lazy_clause(cls, prop, reverse_direction=False):
+        (primaryjoin, secondaryjoin, remote_side) = (prop.polymorphic_primaryjoin, prop.polymorphic_secondaryjoin, prop.remote_side)
+        
         binds = {}
         reverse = {}
 
         def should_bind(targetcol, othercol):
-            return othercol in remote_side
+            if reverse_direction:
+                return targetcol in remote_side
+            else:
+                return othercol in remote_side
 
         def find_column_in_expr(expr):
             if not isinstance(expr, sql.ColumnElement):
@@ -300,9 +305,11 @@ class LazyLoader(AbstractRelationLoader):
             secondaryjoin = secondaryjoin.copy_container()
             lazywhere = sql.and_(lazywhere, secondaryjoin)
  
-        LazyLoader.logger.info(str(self.parent_property) + " lazy loading clause " + str(lazywhere))
+        if hasattr(cls, 'parent_property'):
+            LazyLoader.logger.info(str(cls.parent_property) + " lazy loading clause " + str(lazywhere))
         return (lazywhere, binds, reverse)
-
+    _create_lazy_clause = classmethod(_create_lazy_clause)
+    
 LazyLoader.logger = logging.class_logger(LazyLoader)
 
 

diff --git a/test/orm/mapper.py b/test/orm/mapper.py
index d6563d014b96fb118c497f68f83adc816d8c050b..892f1387429bfe3b05ffbf475d69ac1582fbda7e 100644 (file)
--- a/test/orm/mapper.py
+++ b/test/orm/mapper.py
@@ -387,9 +387,14 @@ class MapperTest(MapperSuperTest):
 
         # test comparing to an object instance
         item = sess.query(Item).get_by(item_name='item 4')
+
+        l = sess.query(Order).select_by(items=item)
+        self.assert_result(l, Order, user_all_result[0]['orders'][1][1])
+
         l = q.select_by(items=item)
         self.assert_result(l, User, user_result[0])
     
+    
         try:
             # this should raise AttributeError
             l = q.select_by(items=5)