Skip to content

Commit

Permalink
Ensure BakedQuery is cloned before we add options to it
Browse files Browse the repository at this point in the history
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
  • Loading branch information
zzzeek committed Jun 26, 2018
1 parent 6778187 commit f243c00
Show file tree
Hide file tree
Showing 4 changed files with 186 additions and 5 deletions.
8 changes: 8 additions & 0 deletions doc/build/changelog/unreleased_12/4286.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.. change::
:tags: bug, orm
:tickets: 4286

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.
7 changes: 7 additions & 0 deletions lib/sqlalchemy/ext/baked.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ def spoil(self, full=False):
self._spoiled = True
return self

def _with_lazyload_options(self, options, effective_path, cache_path=None):
"""Cloning version of _add_lazyload_options.
"""
q = self._clone()
q._add_lazyload_options(options, effective_path, cache_path=cache_path)
return q

def _add_lazyload_options(self, options, effective_path, cache_path=None):
"""Used by per-state lazy loaders to add options to the
"lazy load" query from a parent query.
Expand Down
6 changes: 3 additions & 3 deletions lib/sqlalchemy/orm/loading.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,17 +580,17 @@ def _load_subclass_via_in(context, path, entity):
def do_load(context, path, states, load_only, effective_entity):
orig_query = context.query

q._add_lazyload_options(
q2 = q._with_lazyload_options(
(enable_opt, ) + orig_query._with_options + (disable_opt, ),
path.parent, cache_path=path
)

if orig_query._populate_existing:
q.add_criteria(
q2.add_criteria(
lambda q: q.populate_existing()
)

q(context.session).params(
q2(context.session).params(
primary_keys=[
state.key[1][0] if zero_idx else state.key[1]
for state, load_attrs in states
Expand Down
170 changes: 168 additions & 2 deletions test/orm/inheritance/test_poly_loading.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from sqlalchemy import String, Integer, Column, ForeignKey
from sqlalchemy.orm import relationship, Session, \
selectin_polymorphic, selectinload, with_polymorphic
from sqlalchemy.orm import relationship, Session, joinedload, \
selectin_polymorphic, selectinload, with_polymorphic, backref
from sqlalchemy.testing import fixtures
from sqlalchemy import testing
from sqlalchemy.testing import eq_
Expand Down Expand Up @@ -497,3 +497,169 @@ def test_partial_load_no_invoke_eagers(self):
# the poly loader won't locate a state limited to the "a1" mapper,
# needs to test that it has states
sess.query(a)._with_invoke_all_eagers(False).all()


class LoaderOptionsTest(
fixtures.DeclarativeMappedTest, testing.AssertsExecutionResults):
@classmethod
def setup_classes(cls):
Base = cls.DeclarativeBasic

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

class Child(fixtures.ComparableEntity, 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(50), 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(fixtures.ComparableEntity, 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'))

@classmethod
def insert_data(cls):
Parent, ChildSubclass1, Other = cls.classes(
"Parent", "ChildSubclass1", "Other")
session = Session()

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

def test_options_dont_pollute_baked(self):
self._test_options_dont_pollute(True)

def test_options_dont_pollute_unbaked(self):
self._test_options_dont_pollute(False)

def _test_options_dont_pollute(self, enable_baked):
Parent, ChildSubclass1, Other = self.classes(
"Parent", "ChildSubclass1", "Other")
session = Session(enable_baked_queries=enable_baked)

def no_opt():
q = session.query(Parent).options(
joinedload(Parent.children.of_type(ChildSubclass1)))

return self.assert_sql_execution(
testing.db,
q.all,
CompiledSQL(
"SELECT parent.id AS parent_id, "
"anon_1.child_id AS anon_1_child_id, "
"anon_1.child_parent_id AS anon_1_child_parent_id, "
"anon_1.child_type AS anon_1_child_type, "
"anon_1.child_subclass1_id AS anon_1_child_subclass1_id "
"FROM parent "
"LEFT OUTER JOIN (SELECT child.id AS child_id, "
"child.parent_id AS child_parent_id, "
"child.type AS child_type, "
"child_subclass1.id AS child_subclass1_id "
"FROM child "
"LEFT OUTER JOIN child_subclass1 "
"ON child.id = child_subclass1.id) AS anon_1 "
"ON parent.id = anon_1.child_parent_id",
{}
),
CompiledSQL(
"SELECT child_subclass1.id AS child_subclass1_id, "
"child.id AS child_id, "
"child.parent_id AS child_parent_id, "
"child.type AS child_type "
"FROM child JOIN child_subclass1 "
"ON child.id = child_subclass1.id "
"WHERE child.id IN ([EXPANDING_primary_keys]) "
"ORDER BY child.id",
[{'primary_keys': [1]}]
),
)

result = no_opt()
with self.assert_statement_count(testing.db, 1):
eq_(
result,
[Parent(children=[ChildSubclass1(others=[Other()])])]
)

session.expunge_all()

q = session.query(Parent).options(
joinedload(Parent.children.of_type(ChildSubclass1))
.joinedload(ChildSubclass1.others)
)

result = self.assert_sql_execution(
testing.db,
q.all,
CompiledSQL(
"SELECT parent.id AS parent_id, "
"anon_1.child_id AS anon_1_child_id, "
"anon_1.child_parent_id AS anon_1_child_parent_id, "
"anon_1.child_type AS anon_1_child_type, "
"anon_1.child_subclass1_id AS anon_1_child_subclass1_id, "
"other_1.id AS other_1_id, "
"other_1.child_subclass_id AS other_1_child_subclass_id "
"FROM parent LEFT OUTER JOIN "
"(SELECT child.id AS child_id, "
"child.parent_id AS child_parent_id, "
"child.type AS child_type, "
"child_subclass1.id AS child_subclass1_id "
"FROM child LEFT OUTER JOIN child_subclass1 "
"ON child.id = child_subclass1.id) AS anon_1 "
"ON parent.id = anon_1.child_parent_id "
"LEFT OUTER JOIN other AS other_1 "
"ON anon_1.child_subclass1_id = other_1.child_subclass_id",
{}
),
CompiledSQL(
"SELECT child_subclass1.id AS child_subclass1_id, "
"child.id AS child_id, child.parent_id AS child_parent_id, "
"child.type AS child_type, other_1.id AS other_1_id, "
"other_1.child_subclass_id AS other_1_child_subclass_id "
"FROM child JOIN child_subclass1 "
"ON child.id = child_subclass1.id "
"LEFT OUTER JOIN other AS other_1 "
"ON child_subclass1.id = other_1.child_subclass_id "
"WHERE child.id IN ([EXPANDING_primary_keys]) "
"ORDER BY child.id",
[{'primary_keys': [1]}]
)
)

with self.assert_statement_count(testing.db, 0):
eq_(
result,
[Parent(children=[ChildSubclass1(others=[Other()])])]
)

session.expunge_all()

result = no_opt()
with self.assert_statement_count(testing.db, 1):
eq_(
result,
[Parent(children=[ChildSubclass1(others=[Other()])])]
)

0 comments on commit f243c00

Please sign in to comment.