eager loaders generate incorrect SQL when loading a class mapped to a join

[Christopher Monsanto]

See attached test.

There are three tables "A", "AB", "B", and "C". AB joins A and B.

We desire to relate "A" and "B", but allow the Bs to know what A they came from. To do this, we make a new class BFromA, map it to join(AB, B), and declare a relationship() between A and BFromA.

If you try to joinedload() BFromAs, you get two different possible SQL queries, depending on whether you str() the query, or execute it.

str() gives the correct result:

SELECT a.a_id AS a_a_id, ab_1.b_id AS ab_1_b_id, b_1.b_id AS b_1_b_id, ab_1.a_id AS ab_1_a_id 
FROM a 
LEFT OUTER JOIN (ab AS ab_1 JOIN b AS b_1 ON b_1.b_id = ab_1.b_id) 
ON a.a_id = ab_1.a_id

Executing gives an incorrect result: notice how the joined tables have been changed to a SELECT:

SELECT a.a_id AS a_a_id, anon_1.ab_1_b_id AS ab_1_b_id, anon_1.b_1_b_id AS b_1_b_id, anon_1.ab_1_a_id AS ab_1_a_id 
FROM a 
LEFT OUTER JOIN (SELECT ab_1.a_id AS ab_1_a_id, ab_1.b_id AS ab_1_b_id, b_1.b_id AS b_1_b_id FROM ab AS ab_1 JOIN b AS b_1 ON b_1.b_id = ab_1.b_id) AS anon_1 
ON a.a_id = anon_1.ab_1_a_id

By changing the joined tables to a SELECT, we break any correlated subqueries. We define table C to be a subset of Bs, and add a column_property to BFromA that checks if the current B is in this set. The following is the output of python 3.3 bugtest.py watch as the lazyload and str() joinedload correlate the B correctly with the subquery, and the executed joinedload/subqueryload decorrelate the B.

**Lazy load:

2014-02-19 21:19:24,805 INFO sqlalchemy.engine.base.Engine BEGIN (implicit)
2014-02-19 21:19:24,806 INFO sqlalchemy.engine.base.Engine SELECT ab.b_id AS ab_b_id, b.b_id AS b_b_id, EXISTS (SELECT * 
FROM c 
WHERE c.b_id = b.b_id) AS anon_1, ab.a_id AS ab_a_id 
FROM ab JOIN b ON b.b_id = ab.b_id
 LIMIT ? OFFSET ?
2014-02-19 21:19:24,806 INFO sqlalchemy.engine.base.Engine (1, 0)

**Joined load str():

SELECT a.a_id AS a_a_id, ab_1.b_id AS ab_1_b_id, b_1.b_id AS b_1_b_id, EXISTS (SELECT * 
FROM c 
WHERE c.b_id = b_1.b_id) AS anon_1, ab_1.a_id AS ab_1_a_id 
FROM a LEFT OUTER JOIN (ab AS ab_1 JOIN b AS b_1 ON b_1.b_id = ab_1.b_id) ON a.a_id = ab_1.a_id

**Joined load:

2014-02-19 21:01:50,012 INFO sqlalchemy.engine.base.Engine BEGIN (implicit)
2014-02-19 21:01:50,013 INFO sqlalchemy.engine.base.Engine SELECT a.a_id AS a_a_id, anon_1.ab_1_b_id AS ab_1_b_id, anon_1.b_1_b_id AS b_1_b_id, EXI
STS (SELECT *                                                                                                                                     
FROM c, b AS b_1 
WHERE c.b_id = b_1.b_id) AS anon_2, anon_1.ab_1_a_id AS ab_1_a_id 
FROM a LEFT OUTER JOIN (SELECT ab_1.a_id AS ab_1_a_id, ab_1.b_id AS ab_1_b_id, b_1.b_id AS b_1_b_id 
FROM ab AS ab_1 JOIN b AS b_1 ON b_1.b_id = ab_1.b_id) AS anon_1 ON a.a_id = anon_1.ab_1_a_id
2014-02-19 21:01:50,013 INFO sqlalchemy.engine.base.Engine ()

**Subqueryload:

2014-02-19 21:01:50,015 INFO sqlalchemy.engine.base.Engine SELECT a.a_id AS a_a_id 
FROM a
2014-02-19 21:01:50,015 INFO sqlalchemy.engine.base.Engine ()
2014-02-19 21:01:50,017 INFO sqlalchemy.engine.base.Engine SELECT anon_1.ab_b_id AS ab_b_id, anon_1.b_b_id AS b_b_id, EXISTS (SELECT * 
FROM c, b 
WHERE c.b_id = b.b_id) AS anon_2, anon_1.ab_a_id AS ab_a_id, anon_3.a_a_id AS anon_3_a_a_id 
FROM (SELECT a.a_id AS a_a_id 
FROM a) AS anon_3 JOIN (SELECT ab.a_id AS ab_a_id, ab.b_id AS ab_b_id, b.b_id AS b_b_id 
FROM ab JOIN b ON b.b_id = ab.b_id) AS anon_1 ON anon_3.a_a_id = anon_1.ab_a_id ORDER BY anon_3.a_a_id
2014-02-19 21:01:50,017 INFO sqlalchemy.engine.base.Engine ()

[Christopher Monsanto]

This problem also manifests when you use a composite secondary to relationship(), by the way--the join is turned into a SELECT.

[Mike Bayer]

So the critical point here is to realize that SQLite does not support nesting of joins, nor does it support RIGHT OUTER JOIN. The difference in behavior you are seeing here is due to #2369, and for a full explaination you should read http://docs.sqlalchemy.org/en/rel_0_9/changelog/migration_09.html#many-join-and-left-outer-join-expressions-will-no-longer-be-wrapped-in-select-from-as-anon-1.

So to that extent, the rendering of a new SELECT when a JOIN has to be nested is necessary on SQLite (unless you can show me how to get the syntax mentioned in that migration guide to work on SQLite).

To a deeper extent, the whole "rewriting SQL as SELECT" idea is only present in 0.9. In 0.8, all nested joins are rendered as nested SELECT objects, however the mechanism in 0.8 is ultimately simpler and less error prone than the "rewriting" approach we have in 0.9. I have not dug very deeply into the query here to see if in fact the rewriting is again screwing up (we've already had lots of small issues with the rewriting).

If you want to do some research for me, if you can try out your script on 0.8, which uses the less ideal, but simpler approach of turning nested joins into SELECTs immediately, basically if you can check to see that at least if the results are correct in 0.8 then we will know that the join rewriting in 0.9 has an issue.

[Mike Bayer]

waiting for further testing.

[Christopher Monsanto]

I see your point about parenthesized joins, however, I'm a bit confused about your statement "then we will know that the join rewriting in 0.9 has an issue." You didn't mention correlated subqueries in your reply--did you miss that part? When the joins are rewritten to SELECTs, any correlated subqueries are decorrelated, leading to different results when you lazy load vs when you eager load. So regardless of whether or not we can avoid SELECT rewrites, I'd say there is definitely an issue with the join rewriter. Is it possible to recorrelate any tables present in a join with the new SELECT?

Re: testing, I'm unfortunately under a deadline and can't investigate further at this moment. I was able to work around my problems by avoiding correlation completely:

ab_join = select([AB, B], from_obj=join(AB, B)).alias()

class BFromA(Base):
    __table__ = ab_join

    exists_c = column_property(exists().select_from(C).where(C.b_id == ab_join.c.b_id))

[Mike Bayer]

confirmed this is with join rewriting..

[Mike Bayer]

no i did not miss that part. The approach in 0.8 and in 0.9 towards producing a SELECT here is different. the one in 0.9 is missing the WHERE clause of the EXISTS element, it is not locating the "b_1" which should be replaced by the "anon_1" selectable. that is:

SELECT a.a_id AS a_a_id, anon_1.ab_1_b_id AS ab_1_b_id, anon_1.b_1_b_id AS b_1_b_id, EXISTS (SELECT * 
FROM c, b AS b_1 
WHERE c.b_id = b_1.b_id) AS anon_2, anon_1.ab_1_a_id AS ab_1_a_id 
FROM a LEFT OUTER JOIN (SELECT ab_1.a_id AS ab_1_a_id, ab_1.b_id AS ab_1_b_id, b_1.b_id AS b_1_b_id 
FROM ab AS ab_1 JOIN b AS b_1 ON b_1.b_id = ab_1.b_id) AS anon_1 ON a.a_id = anon_1.ab_1_a_id

should be:

SELECT a.a_id AS a_a_id, anon_1.ab_1_b_id AS ab_1_b_id, anon_1.b_1_b_id AS b_1_b_id, EXISTS (SELECT * 
FROM c 
WHERE c.b_id = anon_1.b_1_b_id) AS anon_2, anon_1.ab_1_a_id AS ab_1_a_id 
FROM a LEFT OUTER JOIN (SELECT ab_1.a_id AS ab_1_a_id, ab_1.b_id AS ab_1_b_id, b_1.b_id AS b_1_b_id 
FROM ab AS ab_1 JOIN b AS b_1 ON b_1.b_id = ab_1.b_id) AS anon_1 ON a.a_id = anon_1.ab_1_a_id

the way correlation works is that when the SELECT statement is embedded in an enclosing SELECT, where the enclosing SELECT features the same FROM object as is present in the inner select, it correlates. There are include/exclude lists available to customize this. But in this case, the "b_1" in the FROM list is due to its presence in the WHERE clause. Translating the column there will fix the issue. It's not clear yet how hard that will be within the current algorithm.

This query is also only failing on SQLite, so I guess that's your target database here.

[Mike Bayer]

here's a patch that looks promising:

diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index d597837..17c9c9e 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -1286,7 +1286,7 @@ class SQLCompiler(Compiled):
         # call down to compiler.visit_join(), compiler.visit_select()
         join_name = selectable.Join.__visit_name__
         select_name = selectable.Select.__visit_name__
-
+        alias_name = selectable.Alias.__visit_name__
         def visit(element, **kw):
             if element in column_translate[-1]:
                 return column_translate[-1][element]
@@ -1307,7 +1307,6 @@ class SQLCompiler(Compiled):
                 selectable_ = selectable.Select(
                                     [right.element],
                                     use_labels=True).alias()
-
                 for c in selectable_.c:
                     c._key_label = c.key
                     c._label = c.name
@@ -1336,7 +1335,8 @@ class SQLCompiler(Compiled):
                 newelem.right = selectable_

                 newelem.onclause = visit(newelem.onclause, **kw)
-            elif newelem.__visit_name__ is select_name:
+            elif newelem.__visit_name__ is alias_name \
+                and newelem.element.__visit_name__ is select_name:
                 column_translate.append({})
                 newelem._copy_internals(clone=visit, **kw)
                 del column_translate[-1]

working on tests now.

[Mike Bayer]

• re: #2967, also fixed a somewhat related issue where join rewriting would fail on the columns clause of the SELECT statement if the targets were aliased tables, as opposed to individual aliased columns.

→ <<cset 69d1d08dc398>>

[Mike Bayer]

fix is in 25831872db7fe2a6eb07c3d50be2504b41d9d5e5. Also spotted a somewhat related issue regarding columns and aliases which isn't likely to occur unless Core is used directly, that is fixed in 69d1d08dc3983b2bba3ed9acafc63a7486ef29c6.

[Mike Bayer]

• More fixes to SQLite "join rewriting"; the fix from 2967 implemented right before the release of 0.9.3 affected the case where a UNION contained nested joins in it. "Join rewriting" is a feature with a wide range of possibilities and is the first intricate "SQL rewriting" feature we've introduced in years, so we're sort of going through a lot of iterations with it (not unlike eager loading back in the 0.2/0.3 series, polymorphic loading in 0.4/0.5). We should be there soon so thanks for bearing with us :). fixes #2969 re: #2967
• solve the issue of join rewriting inspecting various types of from objects without using isinstance(), by adding some new underscored inspection flags to the FromClause hierarchy.

→ <<cset ceaa6047ef8b>>

[Christopher Monsanto]

You're really fast about fixing issues. Thanks!

[Mike Bayer]

these kinds of issues (intricate ORM ones), yes. Firebird datatype reflection, not so much.