- "innerjoin" flag doesn't take effect along the chain

of joinedload() joins if a previous join in that chain
is an outer join, thus allowing primary rows without
a referenced child row to be correctly returned
in results.  [ticket:1954]

diff --git a/CHANGES b/CHANGES
index 452e484a1085fb1a723c542ab104a0f2f816d1fd..65715de54bc60a08607876a6cab183fbe4783e2c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -14,6 +14,12 @@ CHANGES
     that weren't previously saved in the "mutable changes"
     dictionary.
 
+  - "innerjoin" flag doesn't take effect along the chain
+    of joinedload() joins if a previous join in that chain
+    is an outer join, thus allowing primary rows without
+    a referenced child row to be correctly returned
+    in results.  [ticket:1954]
+    
 - mysql
   - Fixed error handling for Jython + zxjdbc, such that 
     has_table() property works again.  Regression from

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 60454eabc27f056c0c5755842536ae164cc023e8..27ad3d902eb5c9b2801b0931a36690172d090852 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -938,6 +938,7 @@ class EagerLoader(AbstractRelationshipLoader):
         
     def setup_query(self, context, entity, path, adapter, \
                                 column_collection=None, parentmapper=None,
+                                allow_innerjoin=True,
                                 **kwargs):
         """Add a left outer join to the statement thats being constructed."""
 
@@ -988,10 +989,18 @@ class EagerLoader(AbstractRelationshipLoader):
             if self.parent_property.direction != interfaces.MANYTOONE:
                 context.multi_row_eager_loaders = True
 
+            innerjoin = allow_innerjoin and context.attributes.get(
+                                ("eager_join_type", path), 
+                                self.parent_property.innerjoin)
+            if not innerjoin:
+                # if this is an outer join, all eager joins from
+                # here must also be outer joins
+                allow_innerjoin = False
+                
             context.create_eager_joins.append(
                 (self._create_eager_join, context, 
                 entity, path, adapter, 
-                parentmapper, clauses)
+                parentmapper, clauses, innerjoin)
             )
 
             add_to_collection = context.secondary_columns
@@ -1006,10 +1015,12 @@ class EagerLoader(AbstractRelationshipLoader):
                 path + (self.mapper,), 
                 clauses, 
                 parentmapper=self.mapper, 
-                column_collection=add_to_collection)
+                column_collection=add_to_collection,
+                allow_innerjoin=allow_innerjoin)
     
     def _create_eager_join(self, context, entity, 
-                            path, adapter, parentmapper, clauses):
+                            path, adapter, parentmapper, 
+                            clauses, innerjoin):
         
         if parentmapper is None:
             localparent = entity.mapper
@@ -1064,10 +1075,6 @@ class EagerLoader(AbstractRelationshipLoader):
         else:
             onclause = self.parent_property
 
-        innerjoin = context.attributes.get(
-                                ("eager_join_type", path), 
-                                self.parent_property.innerjoin)
-
         context.eager_joins[entity_key] = eagerjoin = \
                                 mapperutil.join(
                                             towrap, 

diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index b96014a575257c9289261e2081fe2a2e1f766bca..b70ad0973a38cdc6966d1f14d0ed1ab5d4f1d882 100644 (file)
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -731,6 +731,21 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
             "FROM (SELECT users.id AS users_id, users.name AS users_name "
             "FROM users "
             " LIMIT 10) AS anon_1 LEFT OUTER JOIN orders AS orders_1 ON anon_1.users_id = "
+            "orders_1.user_id LEFT OUTER JOIN addresses AS addresses_1 ON addresses_1.id = orders_1.address_id"
+            ,use_default_dialect=True
+        )
+
+        self.assert_compile(
+            sess.query(User).options(joinedload("orders", innerjoin=True), 
+                                        joinedload("orders.address", innerjoin=True)).limit(10),
+            "SELECT anon_1.users_id AS anon_1_users_id, anon_1.users_name AS anon_1_users_name, "
+            "addresses_1.id AS addresses_1_id, addresses_1.user_id AS addresses_1_user_id, "
+            "addresses_1.email_address AS addresses_1_email_address, orders_1.id AS orders_1_id, "
+            "orders_1.user_id AS orders_1_user_id, orders_1.address_id AS orders_1_address_id, "
+            "orders_1.description AS orders_1_description, orders_1.isopen AS orders_1_isopen "
+            "FROM (SELECT users.id AS users_id, users.name AS users_name "
+            "FROM users "
+            " LIMIT 10) AS anon_1 JOIN orders AS orders_1 ON anon_1.users_id = "
             "orders_1.user_id JOIN addresses AS addresses_1 ON addresses_1.id = orders_1.address_id"
             ,use_default_dialect=True
         )
@@ -919,7 +934,8 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
     @testing.resolve_artifact_names
     def test_inner_join(self):
         mapper(User, users, properties = dict(
-            addresses = relationship(mapper(Address, addresses), lazy='joined', innerjoin=True, order_by=addresses.c.id)
+            addresses = relationship(mapper(Address, addresses), lazy='joined', 
+                                innerjoin=True, order_by=addresses.c.id)
         ))
         sess = create_session()
         eq_(
@@ -938,6 +954,100 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
                 "addresses AS addresses_1 ON users.id = addresses_1.user_id ORDER BY addresses_1.id"
         , use_default_dialect=True)
 
+    @testing.resolve_artifact_names
+    def test_inner_join_chaining_options(self):
+        mapper(User, users, properties = dict(
+            orders =relationship(Order, innerjoin=True, 
+                                    lazy=False)
+        ))
+        mapper(Order, orders, properties=dict(
+            items=relationship(Item, secondary=order_items, lazy=False, 
+                                    innerjoin=True)
+        ))
+        mapper(Item, items)
+        
+        sess = create_session()
+        self.assert_compile(
+            sess.query(User),
+            "SELECT users.id AS users_id, users.name AS users_name, items_1.id AS "
+            "items_1_id, items_1.description AS items_1_description, orders_1.id AS "
+            "orders_1_id, orders_1.user_id AS orders_1_user_id, orders_1.address_id AS "
+            "orders_1_address_id, orders_1.description AS orders_1_description, "
+            "orders_1.isopen AS orders_1_isopen FROM users JOIN orders AS orders_1 ON "
+            "users.id = orders_1.user_id JOIN order_items AS order_items_1 ON orders_1.id = "
+            "order_items_1.order_id JOIN items AS items_1 ON items_1.id = "
+            "order_items_1.item_id",
+            use_default_dialect=True
+        )
+        
+        self.assert_compile(
+            sess.query(User).options(joinedload(User.orders, innerjoin=False)),
+            "SELECT users.id AS users_id, users.name AS users_name, items_1.id AS "
+            "items_1_id, items_1.description AS items_1_description, orders_1.id AS "
+            "orders_1_id, orders_1.user_id AS orders_1_user_id, orders_1.address_id AS "
+            "orders_1_address_id, orders_1.description AS orders_1_description, "
+            "orders_1.isopen AS orders_1_isopen FROM users LEFT OUTER JOIN orders AS orders_1 ON "
+            "users.id = orders_1.user_id LEFT OUTER JOIN order_items AS order_items_1 ON orders_1.id = "
+            "order_items_1.order_id LEFT OUTER JOIN items AS items_1 ON items_1.id = "
+            "order_items_1.item_id",
+            use_default_dialect=True
+        )
+
+        self.assert_compile(
+            sess.query(User).options(joinedload(User.orders, Order.items, innerjoin=False)),
+            "SELECT users.id AS users_id, users.name AS users_name, items_1.id AS "
+            "items_1_id, items_1.description AS items_1_description, orders_1.id AS "
+            "orders_1_id, orders_1.user_id AS orders_1_user_id, orders_1.address_id AS "
+            "orders_1_address_id, orders_1.description AS orders_1_description, "
+            "orders_1.isopen AS orders_1_isopen FROM users JOIN orders AS orders_1 ON "
+            "users.id = orders_1.user_id LEFT OUTER JOIN order_items AS order_items_1 ON orders_1.id = "
+            "order_items_1.order_id LEFT OUTER JOIN items AS items_1 ON items_1.id = "
+            "order_items_1.item_id",
+            use_default_dialect=True
+        )
+        
+    @testing.resolve_artifact_names
+    def test_inner_join_chaining_fixed(self):
+        mapper(User, users, properties = dict(
+            orders =relationship(Order, lazy=False)
+        ))
+        mapper(Order, orders, properties=dict(
+            items=relationship(Item, secondary=order_items, lazy=False, 
+                                    innerjoin=True)
+        ))
+        mapper(Item, items)
+        
+        sess = create_session()
+
+        # joining from user, its all LEFT OUTER JOINs
+        self.assert_compile(
+            sess.query(User),
+            "SELECT users.id AS users_id, users.name AS users_name, items_1.id AS "
+            "items_1_id, items_1.description AS items_1_description, orders_1.id AS "
+            "orders_1_id, orders_1.user_id AS orders_1_user_id, orders_1.address_id AS "
+            "orders_1_address_id, orders_1.description AS orders_1_description, "
+            "orders_1.isopen AS orders_1_isopen FROM users LEFT OUTER JOIN orders AS orders_1 ON "
+            "users.id = orders_1.user_id LEFT OUTER JOIN order_items AS order_items_1 ON orders_1.id = "
+            "order_items_1.order_id LEFT OUTER JOIN items AS items_1 ON items_1.id = "
+            "order_items_1.item_id",
+            use_default_dialect=True
+        )
+        
+        # joining just from Order, innerjoin=True can be respected
+        self.assert_compile(
+            sess.query(Order),
+            "SELECT orders.id AS orders_id, orders.user_id AS orders_user_id, "
+            "orders.address_id AS orders_address_id, orders.description AS "
+            "orders_description, orders.isopen AS orders_isopen, items_1.id "
+            "AS items_1_id, items_1.description AS items_1_description FROM "
+            "orders JOIN order_items AS order_items_1 ON orders.id = "
+            "order_items_1.order_id JOIN items AS items_1 ON items_1.id = "
+            "order_items_1.item_id",
+            use_default_dialect=True
+        )
+        
+        
+
     @testing.resolve_artifact_names
     def test_inner_join_options(self):
         mapper(User, users, properties = dict(