Re: [Sqlalchemy-tickets] [sqlalchemy] #2369: many-to-many + single table joined eager fails since we don't parenthesize joins

[sqlalchemy <mi...@zz...>]

#2369: many-to-many + single table joined eager fails since we don't parenthesize
joins
---------------------------+---------------------------------------
      Reporter:  zzzeek    |      Owner:  zzzeek
          Type:  defect    |     Status:  new
      Priority:  high      |  Milestone:  0.9.0
     Component:  orm       |   Severity:  very major - up to 2 days
    Resolution:            |   Keywords:
Progress State:  in queue  |
---------------------------+---------------------------------------

Comment (by zzzeek):

 so with nested join support working, I thought we could do this:

 {{{
 #!diff
 diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
 index c21e7ea..ff71e39 100644
 --- a/lib/sqlalchemy/orm/util.py
 +++ b/lib/sqlalchemy/orm/util.py
 @@ -916,8 +916,18 @@ class _ORMJoin(expression.Join):
                              of_type=right_info.mapper)

              if sj is not None:
 -                left = sql.join(left, secondary, pj, isouter)
 -                onclause = sj
 +                if not isouter:
 +                    # old way: left [OUTER] JOIN secondary [OUTER] JOIN
 right.
 +                    # for an inner join, this is still OK.
 +                    left = sql.join(left, secondary, pj, isouter)
 +                    onclause = sj
 +                else:
 +                    # new way: left [OUTER] JOIN (secondary INNER JOIN
 right)
 +                    # this is [ticket:2369].  For an outer join, we can
 +                    # get more correctness by using a nested inner
 +                    # for secondary->right
 +                    right = sql.join(secondary, right, sj)
 +                    onclause = pj
              else:
                  onclause = pj
              self._target_adapter = target_adapter
 }}}

 however, this only works for one-level deep eager loading.   the test we
 have like test_froms->SelectFromTest.test_more_joins is doing a huge
 eagerload across two m2ms, and with this it gets the wrong result.  The
 old query there is:

 {{{
 SELECT anon_1.id AS anon_1_id, anon_1.name AS anon_1_name,
         keywords_1.id AS keywords_1_id, keywords_1.name AS
 keywords_1_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 (
         SELECT users.id AS id, users.name AS name
         FROM users
         WHERE users.id IN (%(id_1)s, %(id_2)s)
 ) AS anon_1

         JOIN orders AS orders_2 ON anon_1.id = orders_2.user_id

         JOIN order_items AS order_items_1 ON orders_2.id =
 order_items_1.order_id

         JOIN items AS items_2 ON items_2.id = order_items_1.item_id

         JOIN item_keywords AS item_keywords_1 ON items_2.id =
 item_keywords_1.item_id

         JOIN keywords AS keywords_2 ON keywords_2.id =
 item_keywords_1.keyword_id

         LEFT OUTER JOIN orders AS orders_1 ON anon_1.id = orders_1.user_id

         LEFT OUTER JOIN order_items AS order_items_2 ON orders_1.id =
 order_items_2.order_id

         LEFT OUTER JOIN items AS items_1 ON items_1.id =
 order_items_2.item_id

         LEFT OUTER JOIN item_keywords AS item_keywords_2 ON items_1.id =
 item_keywords_2.item_id

         LEFT OUTER JOIN keywords AS keywords_1 ON keywords_1.id =
 item_keywords_2.keyword_id

 WHERE keywords_2.name IN (%(name_1)s, %(name_2)s, %(name_3)s) ORDER BY
 anon_1.id, orders_1.id, items_1.id, keywords_1.id
 }}}

 the new one is:

 {{{
 SELECT anon_1.id AS anon_1_id, anon_1.name AS anon_1_name,
         keywords_1.id AS keywords_1_id, keywords_1.name AS
 keywords_1_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 (
         SELECT users.id AS id, users.name AS name
         FROM users
         WHERE users.id IN (%(id_1)s, %(id_2)s)
 ) AS anon_1

         JOIN orders AS orders_2 ON anon_1.id = orders_2.user_id

         JOIN order_items AS order_items_1 ON orders_2.id =
 order_items_1.order_id

         JOIN items AS items_2 ON items_2.id = order_items_1.item_id

         JOIN item_keywords AS item_keywords_1 ON items_2.id =
 item_keywords_1.item_id

         JOIN keywords AS keywords_2 ON keywords_2.id =
 item_keywords_1.keyword_id

         LEFT OUTER JOIN orders AS orders_1 ON anon_1.id = orders_1.user_id

         LEFT OUTER JOIN (
                 order_items AS order_items_2 JOIN items AS items_1 ON
 items_1.id = order_items_2.item_id
         ) ON orders_1.id = order_items_2.order_id

         LEFT OUTER JOIN (
                 item_keywords AS item_keywords_2 JOIN keywords AS
 keywords_1 ON keywords_1.id = item_keywords_2.keyword_id
         ) ON items_2.id = item_keywords_2.item_id

 WHERE keywords_2.name IN (%(name_1)s, %(name_2)s, %(name_3)s) ORDER BY
 items_1.id, keywords_1.id
 }}}

 so note, the problem here is that the subsequent eager joins aren't being
 nested.  So we do have to make sure if we do this, we have to change the
 whole way the joins write out for an eager load so that they subsequently
 nest, otherwise we're definitely not getting the right answer.

--
Ticket URL: <http://www.sqlalchemy.org/trac/ticket/2369#comment:7>
sqlalchemy <http://www.sqlalchemy.org/>
The Database Toolkit for Python