From 37c598a41efd2609622b1ca6ee698dbe0ab5ac8b Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 27 Apr 2024 00:31:07 -0400 Subject: [PATCH] ensure intermediary mappers emit subclass IN Fixed issue in :func:`_orm.selectin_polymorhpic` loader option where the SELECT emitted would only accommodate for the child-most class among the result rows that were returned, leading intermediary-class attributes to be unloaded if there were no concrete instances of that intermediary-class present in the result. This issue only presented itself for multi-level inheritance hierarchies. Fixes: #11327 Change-Id: Iec88cc517613d031221a1c035c4cfb46db0154be --- doc/build/changelog/unreleased_20/11327.rst | 10 +++ lib/sqlalchemy/orm/loading.py | 47 +++++++++---- lib/sqlalchemy/orm/mapper.py | 1 + test/orm/inheritance/test_poly_loading.py | 78 ++++++++++++++++++++- 4 files changed, 119 insertions(+), 17 deletions(-) create mode 100644 doc/build/changelog/unreleased_20/11327.rst diff --git a/doc/build/changelog/unreleased_20/11327.rst b/doc/build/changelog/unreleased_20/11327.rst new file mode 100644 index 0000000000..f7169ad980 --- /dev/null +++ b/doc/build/changelog/unreleased_20/11327.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm + :tickets: 11327 + + Fixed issue in :func:`_orm.selectin_polymorhpic` loader option where the + SELECT emitted would only accommodate for the child-most class among the + result rows that were returned, leading intermediary-class attributes to be + unloaded if there were no concrete instances of that intermediary-class + present in the result. This issue only presented itself for multi-level + inheritance hierarchies. diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py index 50258149af..b79bb5fb6f 100644 --- a/lib/sqlalchemy/orm/loading.py +++ b/lib/sqlalchemy/orm/loading.py @@ -1014,21 +1014,38 @@ def _instance_processor( # loading does not apply assert only_load_props is None - callable_ = _load_subclass_via_in( - context, - path, - selectin_load_via, - _polymorphic_from, - option_entities, - ) - PostLoad.callable_for_path( - context, - load_path, - selectin_load_via.mapper, - selectin_load_via, - callable_, - selectin_load_via, - ) + if selectin_load_via.is_mapper: + _load_supers = [] + _endmost_mapper = selectin_load_via + while ( + _endmost_mapper + and _endmost_mapper is not _polymorphic_from + ): + _load_supers.append(_endmost_mapper) + _endmost_mapper = _endmost_mapper.inherits + else: + _load_supers = [selectin_load_via] + + for _selectinload_entity in _load_supers: + if PostLoad.path_exists( + context, load_path, _selectinload_entity + ): + continue + callable_ = _load_subclass_via_in( + context, + path, + _selectinload_entity, + _polymorphic_from, + option_entities, + ) + PostLoad.callable_for_path( + context, + load_path, + _selectinload_entity.mapper, + _selectinload_entity, + callable_, + _selectinload_entity, + ) post_load = PostLoad.for_context(context, load_path, only_load_props) diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index e51ff7df4e..9e1be2fbba 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -3807,6 +3807,7 @@ class Mapper( this subclass as a SELECT with IN. """ + strategy_options = util.preloaded.orm_strategy_options assert self.inherits diff --git a/test/orm/inheritance/test_poly_loading.py b/test/orm/inheritance/test_poly_loading.py index df286f0d35..a768c32754 100644 --- a/test/orm/inheritance/test_poly_loading.py +++ b/test/orm/inheritance/test_poly_loading.py @@ -735,6 +735,66 @@ class TestGeometries(GeometryFixtureBase): with self.assert_statement_count(testing.db, 0): eq_(result, [d(d_data="d1"), e(e_data="e1")]) + @testing.variation("include_intermediary_row", [True, False]) + def test_threelevel_load_only_3lev(self, include_intermediary_row): + """test issue #11327""" + + self._fixture_from_geometry( + { + "a": { + "subclasses": { + "b": {"subclasses": {"c": {}}}, + } + } + } + ) + + a, b, c = self.classes("a", "b", "c") + sess = fixture_session() + sess.add(c(a_data="a1", b_data="b1", c_data="c1")) + if include_intermediary_row: + sess.add(b(a_data="a1", b_data="b1")) + sess.commit() + + sess = fixture_session() + + pks = [] + c_pks = [] + with self.sql_execution_asserter(testing.db) as asserter: + + for obj in sess.scalars( + select(a) + .options(selectin_polymorphic(a, classes=[b, c])) + .order_by(a.id) + ): + assert "b_data" in obj.__dict__ + if isinstance(obj, c): + assert "c_data" in obj.__dict__ + c_pks.append(obj.id) + pks.append(obj.id) + + asserter.assert_( + CompiledSQL( + "SELECT a.id, a.type, a.a_data FROM a ORDER BY a.id", {} + ), + AllOf( + CompiledSQL( + "SELECT c.id AS c_id, b.id AS b_id, a.id AS a_id, " + "a.type AS a_type, c.c_data AS c_c_data FROM a JOIN b " + "ON a.id = b.id JOIN c ON b.id = c.id WHERE a.id IN " + "(__[POSTCOMPILE_primary_keys]) ORDER BY a.id", + [{"primary_keys": c_pks}], + ), + CompiledSQL( + "SELECT b.id AS b_id, a.id AS a_id, a.type AS a_type, " + "b.b_data AS b_b_data FROM a JOIN b ON a.id = b.id " + "WHERE a.id IN (__[POSTCOMPILE_primary_keys]) " + "ORDER BY a.id", + [{"primary_keys": pks}], + ), + ), + ) + @testing.combinations((True,), (False,)) def test_threelevel_selectin_to_inline_awkward_alias_options( self, use_aliased_class @@ -752,7 +812,9 @@ class TestGeometries(GeometryFixtureBase): a, b, c, d, e = self.classes("a", "b", "c", "d", "e") sess = fixture_session() - sess.add_all([d(d_data="d1"), e(e_data="e1")]) + sess.add_all( + [d(c_data="c1", d_data="d1"), e(c_data="c2", e_data="e1")] + ) sess.commit() from sqlalchemy import select @@ -840,6 +902,15 @@ class TestGeometries(GeometryFixtureBase): {}, ), AllOf( + # note this query is added due to the fix made in + # #11327 + CompiledSQL( + "SELECT c.id AS c_id, a.id AS a_id, a.type AS a_type, " + "c.c_data AS c_c_data FROM a JOIN c ON a.id = c.id " + "WHERE a.id IN (__[POSTCOMPILE_primary_keys]) " + "ORDER BY a.id", + [{"primary_keys": [1, 2]}], + ), CompiledSQL( "SELECT d.id AS d_id, c.id AS c_id, a.id AS a_id, " "a.type AS a_type, d.d_data AS d_d_data FROM a " @@ -860,7 +931,10 @@ class TestGeometries(GeometryFixtureBase): ) with self.assert_statement_count(testing.db, 0): - eq_(result, [d(d_data="d1"), e(e_data="e1")]) + eq_( + result, + [d(c_data="c1", d_data="d1"), e(c_data="c2", e_data="e1")], + ) def test_partial_load_no_invoke_eagers(self): # test issue #4199 -- 2.47.2