From: Mike Bayer Date: Sun, 10 Nov 2019 20:42:40 +0000 (-0500) Subject: Exclude local columns when adapting secondary in a join condition X-Git-Tag: rel_1_3_11~4^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7062311ce8da5508a254f51be6922e6b065f28d2;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Exclude local columns when adapting secondary in a join condition Fixed ORM bug where a "secondary" table that referred to a selectable which in some way would refer to the local primary table would apply aliasing to both sides of the join condition when a relationship-related join, either via :meth:`.Query.join` or by :func:`.joinedload`, were generated. The "local" side is now excluded. Fixes: #4974 Change-Id: Ia43da747c22141b05439f4511ddeceb10248582e (cherry picked from commit f3bfe0881726f1b3075400346f5987390ebe6c19) --- diff --git a/doc/build/changelog/unreleased_13/4974.rst b/doc/build/changelog/unreleased_13/4974.rst new file mode 100644 index 0000000000..fc2af4d75d --- /dev/null +++ b/doc/build/changelog/unreleased_13/4974.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, orm + :tickets: 4974 + + Fixed ORM bug where a "secondary" table that referred to a selectable which + in some way would refer to the local primary table would apply aliasing to + both sides of the join condition when a relationship-related join, either + via :meth:`.Query.join` or by :func:`.joinedload`, were generated. The + "local" side is now excluded. diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 1695ba7c98..3b5508ff37 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -3120,7 +3120,6 @@ class JoinCondition(object): criterion, equivalent column sets, etc. """ - # place a barrier on the destination such that # replacement traversals won't ever dig into it. # its internal structure remains fixed @@ -3149,17 +3148,22 @@ class JoinCondition(object): if aliased: if secondary is not None: secondary = secondary.alias(flat=True) - primary_aliasizer = ClauseAdapter(secondary) + primary_aliasizer = ClauseAdapter( + secondary, exclude_fn=_ColInAnnotations("local") + ) secondary_aliasizer = ClauseAdapter( dest_selectable, equivalents=self.child_equivalents ).chain(primary_aliasizer) if source_selectable is not None: - primary_aliasizer = ClauseAdapter(secondary).chain( + primary_aliasizer = ClauseAdapter( + secondary, exclude_fn=_ColInAnnotations("local") + ).chain( ClauseAdapter( source_selectable, equivalents=self.parent_equivalents, ) ) + secondaryjoin = secondary_aliasizer.traverse(secondaryjoin) else: primary_aliasizer = ClauseAdapter( diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py index 0767fbf1c6..2d686c7e90 100644 --- a/test/orm/test_relationships.py +++ b/test/orm/test_relationships.py @@ -27,6 +27,7 @@ from sqlalchemy.orm import mapper from sqlalchemy.orm import relation from sqlalchemy.orm import relationship from sqlalchemy.orm import remote +from sqlalchemy.orm import selectinload from sqlalchemy.orm import Session from sqlalchemy.orm import sessionmaker from sqlalchemy.orm import subqueryload @@ -41,6 +42,8 @@ from sqlalchemy.testing import fixtures from sqlalchemy.testing import in_ from sqlalchemy.testing import is_ from sqlalchemy.testing import startswith_ +from sqlalchemy.testing.assertsql import assert_engine +from sqlalchemy.testing.assertsql import CompiledSQL from sqlalchemy.testing.schema import Column from sqlalchemy.testing.schema import Table from test.orm import _fixtures @@ -4205,7 +4208,6 @@ class SecondaryNestedJoinTest( ) def test_render_lazyload(self): - from sqlalchemy.testing.assertsql import CompiledSQL A = self.classes.A sess = Session() @@ -5474,3 +5476,157 @@ class RelationDeprecationTest(fixtures.MappedTest): session.query(User).filter( User.addresses.any(Address.email_address == "ed@foo.bar") ).one() + + +class SecondaryIncludesLocalColsTest(fixtures.MappedTest): + @classmethod + def define_tables(cls, metadata): + Table( + "a", + metadata, + Column("id", Integer, primary_key=True), + Column("b_ids", String(50)), + ) + + Table("b", metadata, Column("id", String(10), primary_key=True)) + + @classmethod + def setup_classes(cls): + class A(cls.Comparable): + pass + + class B(cls.Comparable): + pass + + @classmethod + def setup_mappers(cls): + A, B = cls.classes("A", "B") + a, b = cls.tables("a", "b") + + secondary = ( + select([a.c.id.label("aid"), b]) + .select_from(a.join(b, a.c.b_ids.like("%" + b.c.id + "%"))) + .alias() + ) + + mapper( + A, + a, + properties=dict( + bs=relationship( + B, + secondary=secondary, + primaryjoin=a.c.id == secondary.c.aid, + secondaryjoin=b.c.id == secondary.c.id, + ) + ), + ) + mapper(B, b) + + @classmethod + def insert_data(cls): + A, B = cls.classes("A", "B") + + s = Session() + s.add_all( + [ + A(id=1, b_ids="1"), + A(id=2, b_ids="2 3"), + B(id="1"), + B(id="2"), + B(id="3"), + ] + ) + s.commit() + + def test_query_join(self): + A, B = self.classes("A", "B") + + s = Session() + + with assert_engine(testing.db) as asserter_: + rows = s.query(A.id, B.id).join(A.bs).order_by(A.id, B.id).all() + + eq_(rows, [(1, "1"), (2, "2"), (2, "3")]) + + asserter_.assert_( + CompiledSQL( + "SELECT a.id AS a_id, b.id AS b_id FROM a JOIN " + "(SELECT a.id AS " + "aid, b.id AS id FROM a JOIN b ON a.b_ids LIKE :id_1 || " + "b.id || :param_1) AS anon_1 ON a.id = anon_1.aid " + "JOIN b ON b.id = anon_1.id ORDER BY a.id, b.id" + ) + ) + + def test_eager_join(self): + A, B = self.classes("A", "B") + + s = Session() + + with assert_engine(testing.db) as asserter_: + a2 = ( + s.query(A).options(joinedload(A.bs)).filter(A.id == 2).all()[0] + ) + + eq_({b.id for b in a2.bs}, {"2", "3"}) + + asserter_.assert_( + CompiledSQL( + "SELECT a.id AS a_id, a.b_ids AS a_b_ids, b_1.id AS b_1_id " + "FROM a LEFT OUTER JOIN ((SELECT a.id AS aid, b.id AS id " + "FROM a JOIN b ON a.b_ids LIKE :id_1 || b.id || :param_1) " + "AS anon_1 JOIN b AS b_1 ON b_1.id = anon_1.id) " + "ON a.id = anon_1.aid WHERE a.id = :id_2", + params=[{"id_1": "%", "param_1": "%", "id_2": 2}], + ) + ) + + def test_exists(self): + A, B = self.classes("A", "B") + + s = Session() + + with assert_engine(testing.db) as asserter_: + eq_(set(id_ for id_, in s.query(A.id).filter(A.bs.any())), {1, 2}) + + asserter_.assert_( + CompiledSQL( + "SELECT a.id AS a_id FROM a WHERE " + "EXISTS (SELECT 1 FROM (SELECT a.id AS aid, b.id AS id " + "FROM a JOIN b ON a.b_ids LIKE :id_1 || b.id || :param_1) " + "AS anon_1, b WHERE a.id = anon_1.aid AND b.id = anon_1.id)", + params=[], + ) + ) + + def test_eager_selectin(self): + A, B = self.classes("A", "B") + + s = Session() + + with assert_engine(testing.db) as asserter_: + a2 = ( + s.query(A) + .options(selectinload(A.bs)) + .filter(A.id == 2) + .all()[0] + ) + + eq_({b.id for b in a2.bs}, {"2", "3"}) + + asserter_.assert_( + CompiledSQL( + "SELECT a.id AS a_id, a.b_ids AS a_b_ids " + "FROM a WHERE a.id = :id_1", + params=[{"id_1": 2}], + ), + CompiledSQL( + "SELECT a_1.id AS a_1_id, b.id AS b_id FROM a AS a_1 JOIN " + "(SELECT a.id AS aid, b.id AS id FROM a JOIN b ON a.b_ids " + "LIKE :id_1 || b.id || :param_1) AS anon_1 " + "ON a_1.id = anon_1.aid JOIN b ON b.id = anon_1.id " + "WHERE a_1.id IN ([EXPANDING_primary_keys]) ORDER BY a_1.id", + params=[{"id_1": "%", "param_1": "%", "primary_keys": [2]}], + ), + )