:ticket:`4993`
:ticket:`4994`
+.. _change_5122:
+
+Stricter behavior when querying inheritance mappings using custom queries
+-------------------------------------------------------------------------
+
+This change applies to the scenario where a joined- or single- table
+inheritance subclass entity is being queried, given a completed SELECT subquery
+to select from. If the given subquery returns rows that do not correspond to
+the requested polymorphic identity or identities, an error is raised.
+Previously, this condition would pass silently under joined table inheritance,
+returning an invalid subclass, and under single table inheritance, the
+:class:`.Query` would be adding additional criteria against the subquery to
+limit the results which could inappropriately interfere with the intent of the
+query.
+
+Given the example mapping of ``Employee``, ``Engineer(Employee)``, ``Manager(Employee)``,
+in the 1.3 series if we were to emit the following query against a joined
+inheritance mapping::
+
+ s = Session(e)
+
+ s.add_all([Engineer(), Manager()])
+
+ s.commit()
+
+ print(
+ s.query(Manager).select_entity_from(s.query(Employee).subquery()).all()
+ )
+
+
+The subquery selects both the ``Engineer`` and the ``Manager`` rows, and
+even though the outer query is against ``Manager``, we get a non ``Manager``
+object back::
+
+ SELECT anon_1.type AS anon_1_type, anon_1.id AS anon_1_id
+ FROM (SELECT employee.type AS type, employee.id AS id
+ FROM employee) AS anon_1
+ 2020-01-29 18:04:13,524 INFO sqlalchemy.engine.base.Engine ()
+ [<__main__.Engineer object at 0x7f7f5b9a9810>, <__main__.Manager object at 0x7f7f5b9a9750>]
+
+The new behavior is that this condition raises an error::
+
+ sqlalchemy.exc.InvalidRequestError: Row with identity key
+ (<class '__main__.Employee'>, (1,), None) can't be loaded into an object;
+ the polymorphic discriminator column '%(140205120401296 anon)s.type'
+ refers to mapped class Engineer->engineer, which is not a sub-mapper of
+ the requested mapped class Manager->manager
+
+The above error only raises if the primary key columns of that entity are
+non-NULL. If there's no primary key for a given entity in a row, no attempt
+to construct an entity is made.
+
+In the case of single inheritance mapping, the change in behavior is slightly
+more involved; if ``Engineer`` and ``Manager`` above are mapped with
+single table inheritance, in 1.3 the following query would be emitted and
+only a ``Manager`` object is returned::
+
+ SELECT anon_1.type AS anon_1_type, anon_1.id AS anon_1_id
+ FROM (SELECT employee.type AS type, employee.id AS id
+ FROM employee) AS anon_1
+ WHERE anon_1.type IN (?)
+ 2020-01-29 18:08:32,975 INFO sqlalchemy.engine.base.Engine ('manager',)
+ [<__main__.Manager object at 0x7ff1b0200d50>]
+
+The :class:`.Query` added the "single table inheritance" criteria to the
+subquery, editorializing on the intent that was originally set up by it.
+This behavior was added in version 1.0 in :ticket:`3891`, and creates a
+behavioral inconsistency between "joined" and "single" table inheritance,
+and additionally modifies the intent of the given query, which may intend
+to return additional rows where the columns that correspond to the inheriting
+entity are NULL, which is a valid use case. The behavior is now equivalent
+to that of joined table inheritance, where it is assumed that the subquery
+returns the correct rows and an error is raised if an unexpected polymorphic
+identity is encountered::
+
+ SELECT anon_1.type AS anon_1_type, anon_1.id AS anon_1_id
+ FROM (SELECT employee.type AS type, employee.id AS id
+ FROM employee) AS anon_1
+ 2020-01-29 18:13:10,554 INFO sqlalchemy.engine.base.Engine ()
+ Traceback (most recent call last):
+ # ...
+ sqlalchemy.exc.InvalidRequestError: Row with identity key
+ (<class '__main__.Employee'>, (1,), None) can't be loaded into an object;
+ the polymorphic discriminator column '%(140700085268432 anon)s.type'
+ refers to mapped class Engineer->employee, which is not a sub-mapper of
+ the requested mapped class Manager->employee
+
+The correct adjustment to the situation as presented above which worked on 1.3
+is to adjust the given subquery to correctly filter the rows based on the
+discriminator column::
+
+ print(
+ s.query(Manager).select_entity_from(
+ s.query(Employee).filter(Employee.discriminator == 'manager').
+ subquery()).all()
+ )
+
+ SELECT anon_1.type AS anon_1_type, anon_1.id AS anon_1_id
+ FROM (SELECT employee.type AS type, employee.id AS id
+ FROM employee
+ WHERE employee.type = ?) AS anon_1
+ 2020-01-29 18:14:49,770 INFO sqlalchemy.engine.base.Engine ('manager',)
+ [<__main__.Manager object at 0x7f70e13fca90>]
+
+
+:ticket:`5122`
+
+
New Features - Core
====================
--- /dev/null
+.. change::
+ :tags: bug, orm
+ :tickets: 5122
+
+ A query that is against a mapped inheritance subclass which also uses
+ :meth:`.Query.select_entity_from` or a similar technique in order to
+ provide an existing subquery to SELECT from, will now raise an error if the
+ given subquery returns entities that do not correspond to the given
+ subclass, that is, they are sibling or superclasses in the same hierarchy.
+ Previously, these would be returned without error. Additionally, if the
+ inheritance mapping is a single-inheritance mapping, the given subquery
+ must apply the appropriate filtering against the polymorphic discriminator
+ column in order to avoid this error; previously, the :class:`.Query` would
+ add this criteria to the outside query however this interferes with some
+ kinds of query that return other kinds of entities as well.
+
+ .. seealso::
+
+ :ref:`change_5122`
\ No newline at end of file
identity_token,
)
if not is_not_primary_key(identitykey[1]):
- raise sa_exc.InvalidRequestError(
- "Row with identity key %s can't be loaded into an "
- "object; the polymorphic discriminator column '%s' is "
- "NULL" % (identitykey, polymorphic_discriminator)
- )
+ return identitykey
+ else:
+ return None
_instance = _decorate_polymorphic_switch(
_instance,
else:
if sub_mapper is mapper:
return None
+ elif not sub_mapper.isa(mapper):
+ return False
return _instance_processor(
sub_mapper,
_instance = polymorphic_instances[discriminator]
if _instance:
return _instance(row)
+ elif _instance is False:
+ identitykey = ensure_no_pk(row)
+
+ if identitykey:
+ raise sa_exc.InvalidRequestError(
+ "Row with identity key %s can't be loaded into an "
+ "object; the polymorphic discriminator column '%s' "
+ "refers to %s, which is not a sub-mapper of "
+ "the requested %s"
+ % (
+ identitykey,
+ polymorphic_on,
+ mapper.polymorphic_map[discriminator],
+ mapper,
+ )
+ )
+ else:
+ return None
else:
return instance_fn(row)
else:
- ensure_no_pk(row)
- return None
+ identitykey = ensure_no_pk(row)
+
+ if identitykey:
+ raise sa_exc.InvalidRequestError(
+ "Row with identity key %s can't be loaded into an "
+ "object; the polymorphic discriminator column '%s' is "
+ "NULL" % (identitykey, polymorphic_on)
+ )
+ else:
+ return None
return polymorphic_instance
self._from_obj_alias = sql_util.ColumnAdapter(
self._from_obj[0], equivs
)
+ self._enable_single_crit = False
elif (
set_base_alias
and len(self._from_obj) == 1
and info.is_aliased_class
):
self._from_obj_alias = info._adapter
+ self._enable_single_crit = False
def _reset_polymorphic_adapter(self, mapper):
for m2 in mapper._with_polymorphic_mappers:
._anonymous_fromclause()
)
q = self._from_selectable(fromclause)
- q._enable_single_crit = False
q._select_from_entity = self._entity_zero()
if entities:
q._set_entities(entities)
):
self.__dict__.pop(attr, None)
self._set_select_from([fromclause], True)
+ self._enable_single_crit = False
# this enables clause adaptation for non-ORM
# expressions.
self._having = criterion
def _set_op(self, expr_fn, *q):
- return self._from_selectable(
- expr_fn(*([self] + list(q))).subquery()
- )._set_enable_single_crit(False)
+ return self._from_selectable(expr_fn(*([self] + list(q))).subquery())
def union(self, *q):
"""Produce a UNION of this Query against one or more queries.
eq_(row, (Parent(id=2), None))
def test_pk_not_null_discriminator_null_from_base(self):
- A, = self.classes("A")
+ (A,) = self.classes("A")
sess = Session()
q = sess.query(A).filter(A.id == 3)
)
def test_pk_not_null_discriminator_null_from_sub(self):
- B, = self.classes("B")
+ (B,) = self.classes("B")
sess = Session()
q = sess.query(B).filter(B.id == 4)
)
+class UnexpectedPolymorphicIdentityTest(fixtures.DeclarativeMappedTest):
+ run_setup_mappers = "once"
+ __dialect__ = "default"
+
+ @classmethod
+ def setup_classes(cls):
+ Base = cls.DeclarativeBasic
+
+ class AJoined(fixtures.ComparableEntity, Base):
+ __tablename__ = "ajoined"
+ id = Column(Integer, primary_key=True)
+ type = Column(String(10), nullable=False)
+ __mapper_args__ = {
+ "polymorphic_on": type,
+ "polymorphic_identity": "a",
+ }
+
+ class AJoinedSubA(AJoined):
+ __tablename__ = "ajoinedsuba"
+ id = Column(ForeignKey("ajoined.id"), primary_key=True)
+ __mapper_args__ = {"polymorphic_identity": "suba"}
+
+ class AJoinedSubB(AJoined):
+ __tablename__ = "ajoinedsubb"
+ id = Column(ForeignKey("ajoined.id"), primary_key=True)
+ __mapper_args__ = {"polymorphic_identity": "subb"}
+
+ class ASingle(fixtures.ComparableEntity, Base):
+ __tablename__ = "asingle"
+ id = Column(Integer, primary_key=True)
+ type = Column(String(10), nullable=False)
+ __mapper_args__ = {
+ "polymorphic_on": type,
+ "polymorphic_identity": "a",
+ }
+
+ class ASingleSubA(ASingle):
+ __mapper_args__ = {"polymorphic_identity": "suba"}
+
+ class ASingleSubB(ASingle):
+ __mapper_args__ = {"polymorphic_identity": "subb"}
+
+ @classmethod
+ def insert_data(cls):
+ ASingleSubA, ASingleSubB, AJoinedSubA, AJoinedSubB = cls.classes(
+ "ASingleSubA", "ASingleSubB", "AJoinedSubA", "AJoinedSubB"
+ )
+ s = Session()
+
+ s.add_all([ASingleSubA(), ASingleSubB(), AJoinedSubA(), AJoinedSubB()])
+ s.commit()
+
+ def test_single_invalid_ident(self):
+ ASingle, ASingleSubA = self.classes("ASingle", "ASingleSubA")
+
+ s = Session()
+
+ q = s.query(ASingleSubA).select_entity_from(
+ select([ASingle]).subquery()
+ )
+
+ assert_raises_message(
+ sa_exc.InvalidRequestError,
+ r"Row with identity key \(.*ASingle.*\) can't be loaded into an "
+ r"object; the polymorphic discriminator column '.*.type' refers "
+ r"to mapped class ASingleSubB->asingle, which is not a "
+ r"sub-mapper of the requested mapped class ASingleSubA->asingle",
+ q.all,
+ )
+
+ def test_joined_invalid_ident(self):
+ AJoined, AJoinedSubA = self.classes("AJoined", "AJoinedSubA")
+
+ s = Session()
+
+ q = s.query(AJoinedSubA).select_entity_from(
+ select([AJoined]).subquery()
+ )
+
+ assert_raises_message(
+ sa_exc.InvalidRequestError,
+ r"Row with identity key \(.*AJoined.*\) can't be loaded into an "
+ r"object; the polymorphic discriminator column '.*.type' refers "
+ r"to mapped class AJoinedSubB->ajoinedsubb, which is not a "
+ r"sub-mapper of the requested mapped class "
+ r"AJoinedSubA->ajoinedsuba",
+ q.all,
+ )
+
+
class NameConflictTest(fixtures.MappedTest):
@classmethod
def define_tables(cls, metadata):
+from sqlalchemy import and_
from sqlalchemy import ForeignKey
from sqlalchemy import func
from sqlalchemy import inspect
class JuniorEngineer(Engineer):
pass
+ class Report(cls.Comparable):
+ pass
+
@classmethod
def setup_mappers(cls):
- Employee, Manager, JuniorEngineer, employees, Engineer = (
+ (
+ Report,
+ reports,
+ Employee,
+ Manager,
+ JuniorEngineer,
+ employees,
+ Engineer,
+ ) = (
+ cls.classes.Report,
+ cls.tables.reports,
cls.classes.Employee,
cls.classes.Manager,
cls.classes.JuniorEngineer,
cls.classes.Engineer,
)
- mapper(Employee, employees, polymorphic_on=employees.c.type)
+ mapper(
+ Report, reports, properties={"employee": relationship(Employee)}
+ )
+ mapper(
+ Employee,
+ employees,
+ polymorphic_on=employees.c.type,
+ properties={
+ "reports": relationship(Report, back_populates="employee")
+ },
+ )
mapper(Manager, inherits=Employee, polymorphic_identity="manager")
mapper(Engineer, inherits=Employee, polymorphic_identity="engineer")
mapper(
sess.add_all([m1, m2, e1, e2])
sess.flush()
+ # note test_basic -> UnexpectedPolymorphicIdentityTest as well
eq_(
sess.query(Manager)
- .select_entity_from(employees.select().limit(10).subquery())
+ .select_entity_from(
+ employees.select()
+ .where(employees.c.type == "manager")
+ .order_by(employees.c.employee_id)
+ .limit(10)
+ .subquery()
+ )
.all(),
[m1, m2],
)
+ def test_select_from_subquery_with_composed_union(self):
+ Report, reports, Manager, JuniorEngineer, employees, Engineer = (
+ self.classes.Report,
+ self.tables.reports,
+ self.classes.Manager,
+ self.classes.JuniorEngineer,
+ self.tables.employees,
+ self.classes.Engineer,
+ )
+
+ sess = create_session()
+ r1, r2, r3, r4 = (
+ Report(name="r1"),
+ Report(name="r2"),
+ Report(name="r3"),
+ Report(name="r4"),
+ )
+ m1 = Manager(name="manager1", manager_data="data1", reports=[r1])
+ m2 = Manager(name="manager2", manager_data="data2", reports=[r2])
+ e1 = Engineer(name="engineer1", engineer_info="einfo1", reports=[r3])
+ e2 = JuniorEngineer(
+ name="engineer2", engineer_info="einfo2", reports=[r4]
+ )
+ sess.add_all([m1, m2, e1, e2])
+ sess.flush()
+
+ stmt = (
+ select([reports, employees])
+ .select_from(
+ reports.outerjoin(
+ employees,
+ and_(
+ employees.c.employee_id == reports.c.employee_id,
+ employees.c.type == "manager",
+ ),
+ )
+ )
+ .apply_labels()
+ .subquery()
+ )
+ eq_(
+ sess.query(Report, Manager)
+ .select_entity_from(stmt)
+ .order_by(Report.name)
+ .all(),
+ [(r1, m1), (r2, m2), (r3, None), (r4, None)],
+ )
+
def test_count(self):
Employee = self.classes.Employee
JuniorEngineer = self.classes.JuniorEngineer
)
def test_type_filtering(self):
- Employee, Manager, reports, Engineer = (
- self.classes.Employee,
+ Report, Manager, Engineer = (
+ self.classes.Report,
self.classes.Manager,
- self.tables.reports,
self.classes.Engineer,
)
- class Report(fixtures.ComparableEntity):
- pass
-
- mapper(
- Report,
- reports,
- properties={"employee": relationship(Employee, backref="reports")},
- )
sess = create_session()
m1 = Manager(name="Tom", manager_data="data1")
)
def test_type_joins(self):
- Employee, Manager, reports, Engineer = (
- self.classes.Employee,
+ Report, Manager, Engineer = (
+ self.classes.Report,
self.classes.Manager,
- self.tables.reports,
self.classes.Engineer,
)
- class Report(fixtures.ComparableEntity):
- pass
-
- mapper(
- Report,
- reports,
- properties={"employee": relationship(Employee, backref="reports")},
- )
sess = create_session()
m1 = Manager(name="Tom", manager_data="data1")
projects / thirdparty / sqlalchemy / sqlalchemy.git / commitdiff
