mapper._subclass_load_via_in() returns mutable state

[sqlalchemy-bot]

Migrated issue, originally created by Michael Bayer (@zzzeek)

demo:

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.declarative import declared_attr

Base = declarative_base()


class Parent(Base):
    __tablename__ = 'parent'
    id = Column(Integer, primary_key=True)


class Child(Base):
    __tablename__ = 'child'
    id = Column(Integer, primary_key=True)
    parent_id = Column(Integer, ForeignKey('parent.id'))
    parent = relationship('Parent', backref=backref('children'))

    type = Column(String(), nullable=False)
    __mapper_args__ = {
        'polymorphic_on': type,
    }


class ChildSubclass1(Child):
    __tablename__ = 'child_subclass1'
    id = Column(Integer, ForeignKey('child.id'), primary_key=True)
    __mapper_args__ = {
        'polymorphic_identity': 'subclass1',
        'polymorphic_load': 'selectin'
    }


class Other(Base):
    __tablename__ = 'other'

    id = Column(Integer, primary_key=True)
    child_subclass_id = Column(Integer, ForeignKey('child_subclass1.id'))
    child_subclass = relationship('ChildSubclass1', backref=backref('others'))

e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)


def is_other_in_session():
    return any(isinstance(model, Other) for model in session)

session = Session(e, enable_baked_queries=False)

parent = Parent()
subclass1 = ChildSubclass1(parent=parent)
other = Other(child_subclass=subclass1)
session.add_all([parent, subclass1, other])
session.commit()


# Test 1: Loading Parent + Children does not load Other
print('\nTest #1: Loading Parent + Children does not load others')

session.expunge_all()
assert not is_other_in_session()
parents = session.query(Parent).options(
    joinedload(Parent.children.of_type(ChildSubclass1))).all()
assert not is_other_in_session()

print('\nTest #2: Loading Parent + Children + Others loads others as expected')
session.expunge_all()
assert not is_other_in_session()
parents = session.query(Parent).options(
    joinedload(Parent.children.of_type(ChildSubclass1))
    .joinedload(ChildSubclass1.others)
).all()

print('\nTest #3: Loading Parent + Children should not load others, '
      'but it does now')
session.expunge_all()
assert not is_other_in_session()

parents = session.query(Parent).options(
    joinedload(Parent.children.of_type(ChildSubclass1))
).all()
assert not is_other_in_session(), 'The above query should not load any Others!'

the options being returned are fixed at _subclass_load_via_in and the options seem to be getting modified in place, test passes as:

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index a30a8c243..17677cb3f 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2814,7 +2814,8 @@ class Mapper(InspectionAttr):
 
         return q, enable_opt, disable_opt
 
-    @_memoized_configured_property
+    #@_memoized_configured_property
+    @property
     def _subclass_load_via_in_mapper(self):
         return self._subclass_load_via_in(self)

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

wip at https://gerrit.sqlalchemy.org/#/c/zzzeek/sqlalchemy/+/791 as the baked cache key is also coming out wrong for that joinedload, this seems like a second issue

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Ensure BakedQuery is cloned before we add options to it

Fixed bug in new polymorphic selectin loading where the BakedQuery used
internally would be mutated by the given loader options, which would both
inappropriately mutate the subclass query as well as carry over the effect
to subsequent queries.

Change-Id: Iaceecb50557f78484d09e55b3029a0483dfe873f
Fixes: #4286

→ f243c00

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Ensure BakedQuery is cloned before we add options to it

Fixed bug in new polymorphic selectin loading where the BakedQuery used
internally would be mutated by the given loader options, which would both
inappropriately mutate the subclass query as well as carry over the effect
to subsequent queries.

Change-Id: Iaceecb50557f78484d09e55b3029a0483dfe873f
Fixes: #4286
(cherry picked from commit f243c00)

→ 199b483