- Added a new option to :paramref:`.relationship.innerjoin` which is

to specify the string ``"nested"``.  When set to ``"nested"`` as opposed
to ``True``, the "chaining" of joins will parenthesize the inner join on the
right side of an existing outer join, instead of chaining as a string
of outer joins.   This possibly should have been the default behavior
when 0.9 was released, as we introduced the feature of right-nested
joins in the ORM, however we are keeping it as a non-default for now
to avoid further surprises.
fixes #2976

5 files changed, 259 insertions, 29 deletions

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 637ef7d86..5f01883b0 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -15,6 +15,19 @@
     :version: 0.9.4
 
     .. change::
+        :tags: orm feature
+        :tickets: 2976
+
+        Added a new option to :paramref:`.relationship.innerjoin` which is
+        to specify the string ``"nested"``.  When set to ``"nested"`` as opposed
+        to ``True``, the "chaining" of joins will parenthesize the inner join on the
+        right side of an existing outer join, instead of chaining as a string
+        of outer joins.   This possibly should have been the default behavior
+        when 0.9 was released, as we introduced the feature of right-nested
+        joins in the ORM, however we are keeping it as a non-default for now
+        to avoid further surprises.
+
+    .. change::
         :tags: bug, ext
         :tickets: 2810
 
diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index 43db14e13..f0e3076ad 100644
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -488,11 +488,22 @@ class RelationshipProperty(StrategizedProperty):
           or when the reference is one-to-one or a collection that is guaranteed
           to have one or at least one entry.
 
+          If the joined-eager load is chained onto an existing LEFT OUTER JOIN,
+          ``innerjoin=True`` will be bypassed and the join will continue to
+          chain as LEFT OUTER JOIN so that the results don't change.  As an alternative,
+          specify the value ``"nested"``.  This will instead nest the join
+          on the right side, e.g. using the form "a LEFT OUTER JOIN (b JOIN c)".
+
+          .. versionadded:: 0.9.4 Added ``innerjoin="nested"`` option to support
+             nesting of eager "inner" joins.
+
           .. seealso::
 
             :ref:`what_kind_of_loading` - Discussion of some details of
             various loader options.
 
+            :parmref:`.joinedload.innerjoin` - loader option version
+
         :param join_depth:
           when non-``None``, an integer value indicating how many levels
           deep "eager" loaders should join on a self-referring or cyclical
@@ -522,8 +533,8 @@ class RelationshipProperty(StrategizedProperty):
 
           * ``joined`` - items should be loaded "eagerly" in the same query as
             that of the parent, using a JOIN or LEFT OUTER JOIN.  Whether
-            the join is "outer" or not is determined by the ``innerjoin``
-            parameter.
+            the join is "outer" or not is determined by the
+            :paramref:`~.relationship.innerjoin` parameter.
 
           * ``subquery`` - items should be loaded "eagerly" as the parents are
             loaded, using one additional SQL statement, which issues a JOIN to a
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 2c18e8129..473b665c8 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -10,6 +10,7 @@
 from .. import exc as sa_exc, inspect
 from .. import util, log, event
 from ..sql import util as sql_util, visitors
+from .. import sql
 from . import (
         attributes, interfaces, exc as orm_exc, loading,
         unitofwork, util as orm_util
@@ -1032,7 +1033,6 @@ class JoinedLoader(AbstractRelationshipLoader):
 
     def setup_query(self, context, entity, path, loadopt, adapter, \
                                 column_collection=None, parentmapper=None,
-                                allow_innerjoin=True,
                                 **kwargs):
         """Add a left outer join to the statement thats being constructed."""
 
@@ -1062,10 +1062,9 @@ class JoinedLoader(AbstractRelationshipLoader):
                 elif path.contains_mapper(self.mapper):
                     return
 
-            clauses, adapter, add_to_collection, \
-                allow_innerjoin = self._generate_row_adapter(
+            clauses, adapter, add_to_collection = self._generate_row_adapter(
                     context, entity, path, loadopt, adapter,
-                    column_collection, parentmapper, allow_innerjoin
+                    column_collection, parentmapper
                 )
 
         with_poly_info = path.get(
@@ -1088,8 +1087,7 @@ class JoinedLoader(AbstractRelationshipLoader):
                 path,
                 clauses,
                 parentmapper=self.mapper,
-                column_collection=add_to_collection,
-                allow_innerjoin=allow_innerjoin)
+                column_collection=add_to_collection)
 
         if with_poly_info is not None and \
             None in set(context.secondary_columns):
@@ -1167,7 +1165,7 @@ class JoinedLoader(AbstractRelationshipLoader):
 
     def _generate_row_adapter(self,
         context, entity, path, loadopt, adapter,
-        column_collection, parentmapper, allow_innerjoin
+        column_collection, parentmapper
     ):
         with_poly_info = path.get(
             context.attributes,
@@ -1189,16 +1187,12 @@ class JoinedLoader(AbstractRelationshipLoader):
         if self.parent_property.direction != interfaces.MANYTOONE:
             context.multi_row_eager_loaders = True
 
-        innerjoin = allow_innerjoin and (
+        innerjoin = (
                             loadopt.local_opts.get(
                                 'innerjoin', self.parent_property.innerjoin)
                             if loadopt is not None
                             else 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,
@@ -1209,7 +1203,7 @@ class JoinedLoader(AbstractRelationshipLoader):
         add_to_collection = context.secondary_columns
         path.set(context.attributes, "eager_row_processor", clauses)
 
-        return clauses, adapter, add_to_collection, allow_innerjoin
+        return clauses, adapter, add_to_collection
 
     def _create_eager_join(self, context, entity,
                             path, adapter, parentmapper,
@@ -1265,13 +1259,34 @@ class JoinedLoader(AbstractRelationshipLoader):
             onclause = self.parent_property
 
         assert clauses.aliased_class is not None
-        context.eager_joins[entity_key] = eagerjoin = \
-                                orm_util.join(
-                                            towrap,
-                                            clauses.aliased_class,
-                                            onclause,
-                                            isouter=not innerjoin
-                                            )
+
+        join_to_outer = innerjoin and isinstance(towrap, sql.Join) and towrap.isouter
+
+        if join_to_outer and innerjoin == 'nested':
+            inner = orm_util.join(
+                                    towrap.right,
+                                    clauses.aliased_class,
+                                    onclause,
+                                    isouter=False
+                                )
+
+            eagerjoin = orm_util.join(
+                                towrap.left,
+                                inner,
+                                towrap.onclause,
+                                isouter=True
+                        )
+            eagerjoin._target_adapter = inner._target_adapter
+        else:
+            if join_to_outer:
+                innerjoin = False
+            eagerjoin = orm_util.join(
+                                        towrap,
+                                        clauses.aliased_class,
+                                        onclause,
+                                        isouter=not innerjoin
+                                        )
+        context.eager_joins[entity_key] = eagerjoin
 
         # send a hint to the Query as to where it may "splice" this join
         eagerjoin.stop_on = entity.selectable
diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index e290e6597..bcd7b76c6 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -609,11 +609,20 @@ def joinedload(loadopt, attr, innerjoin=None):
         # joined-load the keywords collection
         query(Order).options(lazyload(Order.items).joinedload(Item.keywords))
 
-    :func:`.orm.joinedload` also accepts a keyword argument `innerjoin=True` which
-    indicates using an inner join instead of an outer::
+    :param innerjoin: if ``True``, indicates that the joined eager load should
+     use an inner join instead of the default of left outer join::
 
         query(Order).options(joinedload(Order.user, innerjoin=True))
 
+     If the joined-eager load is chained onto an existing LEFT OUTER JOIN,
+     ``innerjoin=True`` will be bypassed and the join will continue to
+     chain as LEFT OUTER JOIN so that the results don't change.  As an alternative,
+     specify the value ``"nested"``.  This will instead nest the join
+     on the right side, e.g. using the form "a LEFT OUTER JOIN (b JOIN c)".
+
+     .. versionadded:: 0.9.4 Added ``innerjoin="nested"`` option to support
+        nesting of eager "inner" joins.
+
     .. note::
 
         The joins produced by :func:`.orm.joinedload` are **anonymously aliased**.
@@ -634,6 +643,11 @@ def joinedload(loadopt, attr, innerjoin=None):
 
         :func:`.orm.lazyload`
 
+        :paramref:`.relationship.lazy`
+
+        :paramref:`.relationship.innerjoin` - :func:`.relationship`-level version
+         of the :paramref:`.joinedload.innerjoin` option.
+
     """
     loader = loadopt.set_relationship_strategy(attr, {"lazy": "joined"})
     if innerjoin is not None:
@@ -680,6 +694,8 @@ def subqueryload(loadopt, attr):
 
         :func:`.orm.lazyload`
 
+        :paramref:`.relationship.lazy`
+
     """
     return loadopt.set_relationship_strategy(attr, {"lazy": "subquery"})
 
@@ -699,6 +715,10 @@ def lazyload(loadopt, attr):
     This function is part of the :class:`.Load` interface and supports
     both method-chained and standalone operation.
 
+    .. seealso::
+
+        :paramref:`.relationship.lazy`
+
     """
     return loadopt.set_relationship_strategy(attr, {"lazy": "select"})
 
@@ -726,6 +746,8 @@ def immediateload(loadopt, attr):
 
         :func:`.orm.lazyload`
 
+        :paramref:`.relationship.lazy`
+
     """
     loader = loadopt.set_relationship_strategy(attr, {"lazy": "immediate"})
     return loader
diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index f2ba3cc27..bfc532576 100644
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -1242,6 +1242,139 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
 
         )
 
+    def test_inner_join_nested_chaining_negative_options(self):
+        users, items, order_items, Order, Item, User, orders = (self.tables.users,
+                                self.tables.items,
+                                self.tables.order_items,
+                                self.classes.Order,
+                                self.classes.Item,
+                                self.classes.User,
+                                self.tables.orders)
+
+        mapper(User, users, properties=dict(
+            orders=relationship(Order, innerjoin='nested',
+                                    lazy=False, order_by=orders.c.id)
+        ))
+        mapper(Order, orders, properties=dict(
+            items=relationship(Item, secondary=order_items, lazy=False,
+                                    innerjoin='nested', order_by=items.c.id)
+        ))
+        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 ORDER BY orders_1.id, items_1.id"
+        )
+
+        q = sess.query(User).options(joinedload(User.orders, innerjoin=False))
+        self.assert_compile(
+            q,
+            "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 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) "
+            "ON users.id = orders_1.user_id ORDER BY orders_1.id, items_1.id"
+        )
+
+        eq_(
+                [
+                    User(id=7,
+                        orders=[
+                            Order(id=1, items=[Item(id=1), Item(id=2), Item(id=3)]),
+                            Order(id=3, items=[Item(id=3), Item(id=4), Item(id=5)]),
+                            Order(id=5, items=[Item(id=5)])]),
+                    User(id=8, orders=[]),
+                    User(id=9, orders=[
+                        Order(id=2, items=[Item(id=1), Item(id=2), Item(id=3)]),
+                        Order(id=4, items=[Item(id=1), Item(id=5)])
+                        ]
+                    ),
+                    User(id=10, orders=[])
+                ],
+            q.order_by(User.id).all()
+        )
+
+        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 "
+            "JOIN items AS items_1 ON items_1.id = order_items_1.item_id) "
+            "ON orders_1.id = order_items_1.order_id ORDER BY orders_1.id, items_1.id"
+
+        )
+
+    def test_inner_join_nested_chaining_positive_options(self):
+        users, items, order_items, Order, Item, User, orders = (self.tables.users,
+                                self.tables.items,
+                                self.tables.order_items,
+                                self.classes.Order,
+                                self.classes.Item,
+                                self.classes.User,
+                                self.tables.orders)
+
+        mapper(User, users, properties=dict(
+            orders=relationship(Order, order_by=orders.c.id)
+        ))
+        mapper(Order, orders, properties=dict(
+            items=relationship(Item, secondary=order_items, order_by=items.c.id)
+        ))
+        mapper(Item, items)
+
+        sess = create_session()
+        q = sess.query(User).options(
+                joinedload("orders", innerjoin=False).\
+                    joinedload("items", innerjoin="nested")
+            )
+
+        self.assert_compile(
+            q,
+            "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 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) ON users.id = orders_1.user_id "
+            "ORDER BY orders_1.id, items_1.id"
+        )
+
+        eq_(
+                [
+                    User(id=7,
+                        orders=[
+                            Order(id=1, items=[Item(id=1), Item(id=2), Item(id=3)]),
+                            Order(id=3, items=[Item(id=3), Item(id=4), Item(id=5)]),
+                            Order(id=5, items=[Item(id=5)])]),
+                    User(id=8, orders=[]),
+                    User(id=9, orders=[
+                        Order(id=2, items=[Item(id=1), Item(id=2), Item(id=3)]),
+                        Order(id=4, items=[Item(id=1), Item(id=5)])
+                        ]
+                    ),
+                    User(id=10, orders=[])
+                ],
+            q.order_by(User.id).all()
+        )
+
     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]
@@ -1259,14 +1392,14 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
                                 self.tables.item_keywords)
 
         mapper(User, users, properties={
-            'orders':relationship(Order, backref='user'), # o2m, m2o
+            'orders': relationship(Order, backref='user'), # o2m, m2o
         })
         mapper(Order, orders, properties={
-            'items':relationship(Item, secondary=order_items,
+            'items': relationship(Item, secondary=order_items,
                                         order_by=items.c.id),  #m2m
         })
         mapper(Item, items, properties={
-            'keywords':relationship(Keyword, secondary=item_keywords,
+            'keywords': relationship(Keyword, secondary=item_keywords,
                                         order_by=keywords.c.id) #m2m
         })
         mapper(Keyword, keywords)
@@ -1309,7 +1442,7 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
                                 self.classes.User,
                                 self.tables.orders)
 
-        mapper(User, users, properties = dict(
+        mapper(User, users, properties=dict(
             orders =relationship(Order, lazy=False)
         ))
         mapper(Order, orders, properties=dict(
@@ -1347,6 +1480,41 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
         )
 
 
+    def test_inner_join_nested_chaining_fixed(self):
+        users, items, order_items, Order, Item, User, orders = (self.tables.users,
+                                self.tables.items,
+                                self.tables.order_items,
+                                self.classes.Order,
+                                self.classes.Item,
+                                self.classes.User,
+                                self.tables.orders)
+
+        mapper(User, users, properties=dict(
+            orders = relationship(Order, lazy=False)
+        ))
+        mapper(Order, orders, properties=dict(
+            items=relationship(Item, secondary=order_items, lazy=False,
+                                    innerjoin='nested')
+        ))
+        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 LEFT OUTER JOIN "
+            "(orders AS orders_1 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) "
+                "ON users.id = orders_1.user_id"
+        )
+
+
 
     def test_inner_join_options(self):
         users, items, order_items, Order, Item, User, orders = (self.tables.users,
@@ -1358,7 +1526,8 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
                                 self.tables.orders)
 
         mapper(User, users, properties = dict(
-            orders =relationship(Order, backref=backref('user', innerjoin=True), order_by=orders.c.id)
+            orders =relationship(Order, backref=backref('user', innerjoin=True),
+                        order_by=orders.c.id)
         ))
         mapper(Order, orders, properties=dict(
             items=relationship(Item, secondary=order_items, order_by=items.c.id)