From 60c36ca8418cec180733a4d97637699fa2d3c36e Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 21 Nov 2015 16:36:50 -0500 Subject: [PATCH] - Fixed joinedload bug which would occur when a. the query includes limit/offset criteria that forces a subquery b. the relationship uses "secondary" c. the primaryjoin of the relationship refers to a column that is either not part of the primary key, or is a PK col in a joined-inheritance subclass table that is under a different attribute name than the parent table's primary key column d. the query defers the columns that are present in the primaryjoin, typically via not being included in load_only(); the necessary column(s) would not be present in the subquery and produce invalid SQL. fixes #3592 --- doc/build/changelog/changelog_10.rst | 15 ++++ lib/sqlalchemy/orm/strategies.py | 3 +- test/orm/test_eager_relations.py | 111 ++++++++++++++++++++++++++- 3 files changed, 126 insertions(+), 3 deletions(-) diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index 40a251a222..7efa3939fb 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -18,6 +18,21 @@ .. changelog:: :version: 1.0.10 + .. change:: + :tags: bug, orm + :versions: 1.1.0b1 + :tickets: 3592 + + Fixed joinedload bug which would occur when a. the query includes + limit/offset criteria that forces a subquery b. the relationship + uses "secondary" c. the primaryjoin of the relationship refers to + a column that is either not part of the primary key, or is a PK + col in a joined-inheritance subclass table that is under a different + attribute name than the parent table's primary key column d. the + query defers the columns that are present in the primaryjoin, typically + via not being included in load_only(); the necessary column(s) would + not be present in the subquery and produce invalid SQL. + .. change:: :tags: bug, orm :versions: 1.1.0b1 diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 67dac1cccb..7de470dd56 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -1367,8 +1367,7 @@ class JoinedLoader(AbstractRelationshipLoader): # send a hint to the Query as to where it may "splice" this join eagerjoin.stop_on = entity.selectable - if self.parent_property.secondary is None and \ - not parentmapper: + if not parentmapper: # for parentclause that is the non-eager end of the join, # ensure all the parent cols in the primaryjoin are actually # in the diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index 084741b27a..1c3b57690a 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -5,7 +5,7 @@ import sqlalchemy as sa from sqlalchemy import testing from sqlalchemy.orm import joinedload, deferred, undefer, \ joinedload_all, backref, Session,\ - defaultload, Load + defaultload, Load, load_only from sqlalchemy import Integer, String, Date, ForeignKey, and_, select, \ func, text from sqlalchemy.testing.schema import Table, Column @@ -4069,3 +4069,112 @@ class CyclicalInheritingEagerTestThree(fixtures.DeclarativeMappedTest, "director_1.id = persistent_1.id) " "ON director.other_id = persistent_1.id" ) + + +class EnsureColumnsAddedTest( + fixtures.DeclarativeMappedTest, testing.AssertsCompiledSQL): + __dialect__ = 'default' + run_create_tables = None + + @classmethod + def setup_classes(cls): + Base = cls.DeclarativeBasic + + class Parent(Base): + __tablename__ = 'parent' + id = Column(Integer, primary_key=True, + test_needs_autoincrement=True) + arb = Column(Integer, unique=True) + data = Column(Integer) + o2mchild = relationship("O2MChild") + m2mchild = relationship("M2MChild", secondary=Table( + 'parent_to_m2m', Base.metadata, + Column('parent_id', ForeignKey('parent.arb')), + Column('child_id', ForeignKey('m2mchild.id')) + )) + + class O2MChild(Base): + __tablename__ = 'o2mchild' + id = Column(Integer, primary_key=True, + test_needs_autoincrement=True) + parent_id = Column(ForeignKey('parent.arb')) + + class M2MChild(Base): + __tablename__ = 'm2mchild' + id = Column(Integer, primary_key=True, + test_needs_autoincrement=True) + + def test_joinedload_defered_pk_limit_o2m(self): + Parent = self.classes.Parent + + s = Session() + + self.assert_compile( + s.query(Parent).options( + load_only('data'), + joinedload(Parent.o2mchild)).limit(10), + "SELECT anon_1.parent_id AS anon_1_parent_id, " + "anon_1.parent_data AS anon_1_parent_data, " + "anon_1.parent_arb AS anon_1_parent_arb, " + "o2mchild_1.id AS o2mchild_1_id, " + "o2mchild_1.parent_id AS o2mchild_1_parent_id " + "FROM (SELECT parent.id AS parent_id, parent.data AS parent_data, " + "parent.arb AS parent_arb FROM parent LIMIT :param_1) AS anon_1 " + "LEFT OUTER JOIN o2mchild AS o2mchild_1 " + "ON anon_1.parent_arb = o2mchild_1.parent_id" + ) + + def test_joinedload_defered_pk_limit_m2m(self): + Parent = self.classes.Parent + + s = Session() + + self.assert_compile( + s.query(Parent).options( + load_only('data'), + joinedload(Parent.m2mchild)).limit(10), + "SELECT anon_1.parent_id AS anon_1_parent_id, " + "anon_1.parent_data AS anon_1_parent_data, " + "anon_1.parent_arb AS anon_1_parent_arb, " + "m2mchild_1.id AS m2mchild_1_id " + "FROM (SELECT parent.id AS parent_id, " + "parent.data AS parent_data, parent.arb AS parent_arb " + "FROM parent LIMIT :param_1) AS anon_1 " + "LEFT OUTER JOIN (parent_to_m2m AS parent_to_m2m_1 " + "JOIN m2mchild AS m2mchild_1 " + "ON m2mchild_1.id = parent_to_m2m_1.child_id) " + "ON anon_1.parent_arb = parent_to_m2m_1.parent_id" + ) + + def test_joinedload_defered_pk_o2m(self): + Parent = self.classes.Parent + + s = Session() + + self.assert_compile( + s.query(Parent).options( + load_only('data'), + joinedload(Parent.o2mchild)), + "SELECT parent.id AS parent_id, parent.data AS parent_data, " + "parent.arb AS parent_arb, o2mchild_1.id AS o2mchild_1_id, " + "o2mchild_1.parent_id AS o2mchild_1_parent_id " + "FROM parent LEFT OUTER JOIN o2mchild AS o2mchild_1 " + "ON parent.arb = o2mchild_1.parent_id" + ) + + def test_joinedload_defered_pk_m2m(self): + Parent = self.classes.Parent + + s = Session() + + self.assert_compile( + s.query(Parent).options( + load_only('data'), + joinedload(Parent.m2mchild)), + "SELECT parent.id AS parent_id, parent.data AS parent_data, " + "parent.arb AS parent_arb, m2mchild_1.id AS m2mchild_1_id " + "FROM parent LEFT OUTER JOIN (parent_to_m2m AS parent_to_m2m_1 " + "JOIN m2mchild AS m2mchild_1 " + "ON m2mchild_1.id = parent_to_m2m_1.child_id) " + "ON parent.arb = parent_to_m2m_1.parent_id" + ) -- 2.47.2