- Fixed a regression caused by :ticket:`2976` released in 0.9.4 where

the "outer join" propagation along a chain of joined eager loads
would incorrectly convert an "inner join" along a sibling join path
into an outer join as well, when only descendant paths should be
receiving the "outer join" propagation; additionally, fixed related
issue where "nested" join propagation would take place inappropriately
between two sibling join paths.

this is accomplished by re-introducing the removed flag "allow_innerjoin",
now inverted and named "chained_from_outerjoin".  Propagating this flag
allows us to know when we have encountered an outerjoin along a load
path, without confusing it for state obtained from a sibling path.

fixes #3131
ref #2976

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index cd15d289b3f3ea829cd5802b1b063a1227782820..4a37af3cf5a7017d2eac7ed50cb3b5d3ff479556 100644 (file)
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -14,6 +14,19 @@
     :version: 0.9.7
     :released:
 
+    .. change::
+        :tags: bug, orm, eagerloading
+        :tickets: 3131
+        :versions: 1.0.0
+
+        Fixed a regression caused by :ticket:`2976` released in 0.9.4 where
+        the "outer join" propagation along a chain of joined eager loads
+        would incorrectly convert an "inner join" along a sibling join path
+        into an outer join as well, when only descendant paths should be
+        receiving the "outer join" propagation; additionally, fixed related
+        issue where "nested" join propagation would take place inappropriately
+        between two sibling join paths.
+
     .. change::
         :tags: bug, sqlite
         :tickets: 3130

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index f2d4935d74b469d858fd25b73257908146a66c63..b9de68f4b6db66ad7dba2edff30e64e5daa0189d 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -1047,6 +1047,7 @@ class JoinedLoader(AbstractRelationshipLoader):
 
     def setup_query(self, context, entity, path, loadopt, adapter, \
                                 column_collection=None, parentmapper=None,
+                                chained_from_outerjoin=False,
                                 **kwargs):
         """Add a left outer join to the statement that's being constructed."""
 
@@ -1076,10 +1077,11 @@ class JoinedLoader(AbstractRelationshipLoader):
                 elif path.contains_mapper(self.mapper):
                     return
 
-            clauses, adapter, add_to_collection = self._generate_row_adapter(
-                    context, entity, path, loadopt, adapter,
-                    column_collection, parentmapper
-                )
+            clauses, adapter, add_to_collection, chained_from_outerjoin = \
+                self._generate_row_adapter(
+                        context, entity, path, loadopt, adapter,
+                        column_collection, parentmapper, chained_from_outerjoin
+                    )
 
         with_poly_info = path.get(
             context.attributes,
@@ -1101,7 +1103,8 @@ class JoinedLoader(AbstractRelationshipLoader):
                 path,
                 clauses,
                 parentmapper=self.mapper,
-                column_collection=add_to_collection)
+                column_collection=add_to_collection,
+                chained_from_outerjoin=chained_from_outerjoin)
 
         if with_poly_info is not None and \
             None in set(context.secondary_columns):
@@ -1179,7 +1182,7 @@ class JoinedLoader(AbstractRelationshipLoader):
 
     def _generate_row_adapter(self,
         context, entity, path, loadopt, adapter,
-        column_collection, parentmapper
+        column_collection, parentmapper, chained_from_outerjoin
     ):
         with_poly_info = path.get(
             context.attributes,
@@ -1208,20 +1211,25 @@ class JoinedLoader(AbstractRelationshipLoader):
                             else self.parent_property.innerjoin
                         )
 
+        if not innerjoin:
+            # if this is an outer join, all non-nested eager joins from
+            # this path must also be outer joins
+            chained_from_outerjoin = True
+
         context.create_eager_joins.append(
             (self._create_eager_join, context,
             entity, path, adapter,
-            parentmapper, clauses, innerjoin)
+            parentmapper, clauses, innerjoin, chained_from_outerjoin)
         )
 
         add_to_collection = context.secondary_columns
         path.set(context.attributes, "eager_row_processor", clauses)
 
-        return clauses, adapter, add_to_collection
+        return clauses, adapter, add_to_collection, chained_from_outerjoin
 
     def _create_eager_join(self, context, entity,
                             path, adapter, parentmapper,
-                            clauses, innerjoin):
+                            clauses, innerjoin, chained_from_outerjoin):
 
         if parentmapper is None:
             localparent = entity.mapper
@@ -1276,7 +1284,7 @@ class JoinedLoader(AbstractRelationshipLoader):
 
         join_to_outer = innerjoin and isinstance(towrap, sql.Join) and towrap.isouter
 
-        if join_to_outer and innerjoin == 'nested':
+        if chained_from_outerjoin and join_to_outer and innerjoin == 'nested':
             inner = orm_util.join(
                                     towrap.right,
                                     clauses.aliased_class,
@@ -1292,7 +1300,7 @@ class JoinedLoader(AbstractRelationshipLoader):
                         )
             eagerjoin._target_adapter = inner._target_adapter
         else:
-            if join_to_outer:
+            if chained_from_outerjoin:
                 innerjoin = False
             eagerjoin = orm_util.join(
                                         towrap,

diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 7d1f79e97d3e9df23b8a30908a6e33be2d07bb04..8e09d60764f3dd71655d46a717d64e5f01c65e14 100644 (file)
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -1375,6 +1375,78 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
             q.order_by(User.id).all()
         )
 
+    def test_unnested_outerjoin_propagation_only_on_correct_path(self):
+        # test #3131
+
+        User, users = self.classes.User, self.tables.users
+        Order, orders = self.classes.Order, self.tables.orders
+        Address, addresses = self.classes.Address, self.tables.addresses
+
+        mapper(User, users, properties={
+            'orders': relationship(Order),
+            'addresses': relationship(Address)
+        })
+        mapper(Order, orders)
+        mapper(Address, addresses)
+
+        sess = create_session()
+        q = sess.query(User).options(
+            joinedload("orders"),
+            joinedload("addresses", innerjoin=True),
+        )
+
+        self.assert_compile(
+            q,
+            "SELECT users.id AS users_id, users.name AS users_name, "
+            "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, "
+            "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 "
+            "FROM users LEFT OUTER JOIN orders AS orders_1 "
+            "ON users.id = orders_1.user_id JOIN addresses AS addresses_1 "
+            "ON users.id = addresses_1.user_id"
+        )
+
+    def test_nested_outerjoin_propagation_only_on_correct_path(self):
+        # test #3131
+
+        User, users = self.classes.User, self.tables.users
+        Order, orders = self.classes.Order, self.tables.orders
+        Address, addresses = self.classes.Address, self.tables.addresses
+
+        mapper(User, users, properties={
+            'orders': relationship(Order),
+            'addresses': relationship(Address)
+        })
+        mapper(Order, orders)
+        mapper(Address, addresses)
+
+        sess = create_session()
+        q = sess.query(User).options(
+            joinedload("orders"),
+            joinedload("addresses", innerjoin='nested'),
+        )
+
+        self.assert_compile(
+            q,
+            "SELECT users.id AS users_id, users.name AS users_name, "
+            "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, "
+            "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 "
+            "FROM users LEFT OUTER JOIN orders AS orders_1 "
+            "ON users.id = orders_1.user_id JOIN addresses AS addresses_1 "
+            "ON users.id = addresses_1.user_id"
+        )
+
     def test_catch_the_right_target(self):
         # test eager join chaining to the "nested" join on the left,
         # a new feature as of [ticket:2369]