Join rewriting (?maybe?) issue (union and joined table inheritance involved)

[Jeff Dairiki]

This is a regression in 0.9.4. I suspect this is an issue only with sqlite. A test script which exercises the problem is attached.

Symptoms

With

class A(Base):
    __tablename__ = 'A'
    id = sa.Column(sa.Integer, primary_key=True)
    typ = sa.Column(sa.Integer, nullable=False)
    __mapper_args__ = {'polymorphic_on': typ}
    b_id = sa.Column(sa.ForeignKey('B.id'), nullable=False)

class A1(A):
    __tablename__ = 'A1'
    __mapper_args__ = {'polymorphic_identity': 1}
    id = sa.Column(sa.ForeignKey(A.id), primary_key=True)

class A2(A):
    __tablename__ = 'A2'
    __mapper_args__ = {'polymorphic_identity': 2}
    id = sa.Column(sa.ForeignKey(A.id), primary_key=True)

class B(Base):
    __tablename__ = 'B'
    id = sa.Column(sa.Integer, primary_key=True)
    a1s = sa.orm.relationship(A1)
    a2s = sa.orm.relationship(A2)

sess.query(B).join(B.a1s).union(sess.query(B).join(B.a2s)) compiles erroneously to

SELECT anon_1."B_id" AS "anon_1_B_id" FROM (
  SELECT "B".id AS "B_id" 
    FROM "B"
     JOIN (
       SELECT "A".id AS "A_id",
              "A".typ AS "A_typ",
              "A".b_id AS "A_b_id",
              "A1".id AS "A1_id" 
         FROM "A" JOIN "A1" ON "A".id = "A1".id
     ) AS anon_2 ON "B".id = anon_2."A_b_id"
  UNION SELECT "B".id AS "B_id" 
    FROM "B"
     JOIN (
       SELECT anon_2."A_id" AS "A_id", 
              anon_2."A_typ" AS "A_typ",
              anon_2."A_b_id" AS "A_b_id",
              anon_2."A1_id" AS "A1_id",
              "A2".id AS "A2_id" 
         FROM (
           SELECT "A".id AS "A_id",
                  "A".typ AS "A_typ",
                  "A".b_id AS "A_b_id",
                  "A1".id AS "A1_id" 
             FROM "A" JOIN "A1" ON "A".id = "A1".id
         ) AS anon_2
          JOIN "A2" ON anon_2."A_id" = "A2".id
      ) AS anon_3 ON "B".id = anon_3."A_b_id"
) AS anon_1

Note that in the the second part of the union (the part corresponding to sess.query(B).join(B.a2s) is effectively joining A with both A1 and A2 (which always results in zero rows, since an A is either an A1 or an A2, never both.)

[Mike Bayer]

and not in 0.9.3 huh. OK. well we know where it is anyway (and yes these are only sqlite. because they wont add an incredibly simple syntax that even mysql does).

[Jeff Dairiki]

Sorry — typo/braino. The regression is in 0.9.3.

[Mike Bayer]

OK so it breaks as of 0.9.3? not in 0.9.2 then? sorry havent just run it yet

[Mike Bayer]

oh duh. right we're only on 0.9.3. OK.

[Jeff Dairiki]

Yes. To be specific, 2583187 seems to be the culprit. (That is, the attached script breaks on 2583187 and not on its parent a0fb9e7.)

[Mike Bayer]

ok well here's your patch, I need to translate the query into test_join_rewriting:

diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 148da19..af790a4 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -1287,6 +1287,8 @@ class SQLCompiler(Compiled):
         join_name = selectable.Join.__visit_name__
         select_name = selectable.Select.__visit_name__
         alias_name = selectable.Alias.__visit_name__
+        compound_select_name = selectable.CompoundSelect.__visit_name__
+
         def visit(element, **kw):
             if element in column_translate[-1]:
                 return column_translate[-1][element]
@@ -1340,11 +1342,17 @@ class SQLCompiler(Compiled):
                 newelem.right = selectable_

                 newelem.onclause = visit(newelem.onclause, **kw)
-            elif newelem.__visit_name__ is alias_name \
-                and newelem.element.__visit_name__ is select_name:
-                column_translate.append({})
+            elif newelem.__visit_name__ in (alias_name, compound_select_name):
+                kw['transform_clue'] = 'select_container'
+                newelem._copy_internals(clone=visit, **kw)
+            elif newelem.__visit_name__ is select_name:
+                barrier_select = kw.get('transform_clue', None) == 'select_container'
+                if barrier_select:
+                    column_translate.append({})
+                kw['transform_clue'] = 'inside_select'
                 newelem._copy_internals(clone=visit, **kw)
-                del column_translate[-1]
+                if barrier_select:
+                    del column_translate[-1]
             else:
                 newelem._copy_internals(clone=visit, **kw)

[Jeff Dairiki]

Yes, that fixes the issue here. (Not only does the attached test script pass, but so does the complete test suite for my big hairy app.)

[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>>

[Jeff Dairiki]

I've just verified that ceaa604 passes all the tests I have here.

Thank you for another quick fix, Mike!