]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
remove "deannotate" from column_property expression
authorMike Bayer <mike_mp@zzzcomputing.com>
Fri, 27 May 2022 20:07:01 +0000 (16:07 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Sun, 29 May 2022 20:10:54 +0000 (16:10 -0400)
Fixed issue where using a :func:`_orm.column_property` construct containing
a subquery against an already-mapped column attribute would not correctly
apply ORM-compilation behaviors to the subquery, including that the "IN"
expression added for a single-table inherits expression would fail to be
included.

This fix involves a few tweaks in the ORM adaptation logic,
including a missing "parententity" adaptation on the mapper
side.  The specific mechanics here have a lot of moving parts
so we will continue to add tests to assert these cases.  In
particular a more complete test for issue #2316 is added
that was relying upon the deannotate happening here.

Fixes: #8064
Change-Id: Ia85dd12dcf6e7c002b30de4a27d7aa66cb3cd20e

doc/build/changelog/unreleased_14/8064.rst [new file with mode: 0644]
lib/sqlalchemy/orm/mapper.py
lib/sqlalchemy/orm/properties.py
lib/sqlalchemy/orm/util.py
test/orm/inheritance/test_single.py
test/orm/test_eager_relations.py
test/orm/test_mapper.py

diff --git a/doc/build/changelog/unreleased_14/8064.rst b/doc/build/changelog/unreleased_14/8064.rst
new file mode 100644 (file)
index 0000000..ccac2ad
--- /dev/null
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 8064
+
+    Fixed issue where using a :func:`_orm.column_property` construct containing
+    a subquery against an already-mapped column attribute would not correctly
+    apply ORM-compilation behaviors to the subquery, including that the "IN"
+    expression added for a single-table inherits expression would fail to be
+    included.
index 2d3bceb92807df9a3c16b6d3f64166af47ac29d5..2a227f9daa38eda1b9c6c7476ba49960d6de72fd 100644 (file)
@@ -1911,7 +1911,7 @@ class Mapper(
 
             self.columns.add(col, key)
 
-            for col in prop.columns + prop._orig_columns:
+            for col in prop.columns:
                 for proxy_col in col.proxy_set:
                     self._columntoproperty[proxy_col] = prop
 
@@ -2260,9 +2260,9 @@ class Mapper(
     @HasMemoized.memoized_attribute
     def _single_table_criterion(self):
         if self.single and self.inherits and self.polymorphic_on is not None:
-            return self.polymorphic_on._annotate({"parentmapper": self}).in_(
-                [m.polymorphic_identity for m in self.self_and_descendants]
-            )
+            return self.polymorphic_on._annotate(
+                {"parententity": self, "parentmapper": self}
+            ).in_([m.polymorphic_identity for m in self.self_and_descendants])
         else:
             return None
 
index 7655f3ae2f79dea7c259c8c76db31bd9538b44d4..d77d6e63c0e776b5d7859e16ab7bca75cdfc69c8 100644 (file)
@@ -38,7 +38,6 @@ from .interfaces import MapperProperty
 from .interfaces import PropComparator
 from .interfaces import StrategizedProperty
 from .relationships import Relationship
-from .util import _orm_full_deannotate
 from .. import exc as sa_exc
 from .. import ForeignKey
 from .. import log
@@ -103,7 +102,6 @@ class ColumnProperty(
     _links_to_entity = False
 
     columns: List[NamedColumn[Any]]
-    _orig_columns: List[NamedColumn[Any]]
 
     _is_polymorphic_discriminator: bool
 
@@ -112,7 +110,6 @@ class ColumnProperty(
     comparator_factory: Type[PropComparator[_T]]
 
     __slots__ = (
-        "_orig_columns",
         "columns",
         "group",
         "deferred",
@@ -148,14 +145,8 @@ class ColumnProperty(
             attribute_options=attribute_options
         )
         columns = (column,) + additional_columns
-        self._orig_columns = [
-            coercions.expect(roles.LabeledColumnExprRole, c) for c in columns
-        ]
         self.columns = [
-            _orm_full_deannotate(
-                coercions.expect(roles.LabeledColumnExprRole, c)
-            )
-            for c in columns
+            coercions.expect(roles.LabeledColumnExprRole, c) for c in columns
         ]
         self.group = group
         self.deferred = deferred
index 520c95672f42c97c9a94ccd21b95b76513e9edd1..c8c802d9f24853fb8e9b063ba38251a5219c471a 100644 (file)
@@ -475,7 +475,7 @@ class ORMAdapter(sql_util.ColumnAdapter):
 
     def _include_fn(self, elem):
         entity = elem._annotations.get("parentmapper", None)
-        return not entity or entity.isa(self.mapper)
+        return not entity or entity.common_parent(self.mapper)
 
 
 class AliasedClass(
index efb3c9979849a65011ba5c8e3b87e3876a8e9e50..86422000f7f014bb5b083c5e8c0e4941dcd4d01e 100644 (file)
@@ -13,6 +13,7 @@ from sqlalchemy import testing
 from sqlalchemy import true
 from sqlalchemy.orm import aliased
 from sqlalchemy.orm import Bundle
+from sqlalchemy.orm import column_property
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm import relationship
 from sqlalchemy.orm import Session
@@ -1935,3 +1936,35 @@ class EagerDefaultEvalTestPolymorphic(EagerDefaultEvalTest):
         super(EagerDefaultEvalTestPolymorphic, cls).setup_classes(
             with_polymorphic="*"
         )
+
+
+class ColExprTest(AssertsCompiledSQL, fixtures.TestBase):
+    def test_discrim_on_column_prop(self, registry):
+        Base = registry.generate_base()
+
+        class Employee(Base):
+            __tablename__ = "employee"
+            id = Column(Integer, primary_key=True)
+            type = Column(String(20))
+
+            __mapper_args__ = {
+                "polymorphic_on": "type",
+                "polymorphic_identity": "employee",
+            }
+
+        class Engineer(Employee):
+            __mapper_args__ = {"polymorphic_identity": "engineer"}
+
+        class Company(Base):
+            __tablename__ = "company"
+            id = Column(Integer, primary_key=True)
+
+            max_engineer_id = column_property(
+                select(func.max(Engineer.id)).scalar_subquery()
+            )
+
+        self.assert_compile(
+            select(Company.max_engineer_id),
+            "SELECT (SELECT max(employee.id) AS max_1 FROM employee "
+            "WHERE employee.type IN (__[POSTCOMPILE_type_1])) AS anon_1",
+        )
index 8c82450da0efc6ebf637841d8b0bf8f1bcacba0d..a5bd4c75a28935e0a9094b3c5140b3b06c306552 100644 (file)
@@ -179,6 +179,37 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
         # has to lazy load the addresses
         self.assert_sql_count(testing.db, go, 1)
 
+    def test_column_property_adaptation(self, decl_base):
+        """test #2316 in support of #8064"""
+
+        class A(decl_base):
+            __tablename__ = "a"
+            id = Column(Integer, primary_key=True)
+            type = Column(String(40), nullable=False)
+            __mapper_args__ = {"polymorphic_on": type}
+
+        A.anything = column_property(A.id + 1000)
+
+        class B(A):
+            __tablename__ = "b"
+            account_id = Column(Integer, ForeignKey("a.id"), primary_key=True)
+            x_id = Column(Integer, ForeignKey("x.id"), nullable=False)
+            __mapper_args__ = {"polymorphic_identity": "named"}
+
+        class X(decl_base):
+            __tablename__ = "x"
+            id = Column(Integer, primary_key=True)
+            b = relationship("B")
+
+        self.assert_compile(
+            select(X).options(joinedload(X.b)),
+            "SELECT x.id, a_1.id AS id_1, a_1.type, a_1.id + :id_2 AS anon_1, "
+            "b_1.account_id, b_1.x_id FROM x "
+            "LEFT OUTER JOIN "
+            "(a AS a_1 JOIN b AS b_1 ON a_1.id = b_1.account_id) "
+            "ON x.id = b_1.x_id",
+        )
+
     def test_no_render_in_subquery(self):
         """test #6378"""
 
index d8cc48939b0fe616cf0b17c7b405c578ee6f6984..f11ad31009d05224773a09e2a37c7d3142953a16 100644 (file)
@@ -879,7 +879,8 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL):
         )
 
     @testing.combinations((True,), (False,))
-    def test_add_column_prop_deannotate(self, autoalias):
+    def test_add_column_prop_adaption(self, autoalias):
+        """test ultimately from #2316 revised for #8064"""
         User, users = self.classes.User, self.tables.users
         Address, addresses = self.classes.Address, self.tables.addresses
 
@@ -940,9 +941,13 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL):
                 "users_1.id = addresses.user_id",
             )
 
-    def test_column_prop_deannotate(self):
-        """test that column property deannotates,
-        bringing expressions down to the original mapped columns.
+    def test_column_prop_stays_annotated(self):
+        """test ultimately from #2316 revised for #8064.
+
+        previously column_property() would deannotate the given expression,
+        however this interfered with some compilation sceanrios.
+
+
         """
         User, users = self.classes.User, self.tables.users
         m = self.mapper(User, users)
@@ -954,14 +959,18 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL):
         m.add_property("y", column_property(expr2.scalar_subquery()))
 
         assert User.x.property.columns[0] is not expr
-        assert User.x.property.columns[0].element.left is users.c.name
-        # a deannotate needs to clone the base, in case
-        # the original one referenced annotated elements.
-        assert User.x.property.columns[0].element.right is not expr.right
+
+        assert (
+            User.x.property.columns[0].element.left
+            is User.name.comparator.expr
+        )
+
+        assert User.x.property.columns[0].element.right is expr.right
 
         assert User.y.property.columns[0] is not expr2
         assert (
-            User.y.property.columns[0].element._raw_columns[0] is users.c.name
+            User.y.property.columns[0].element._raw_columns[0]
+            is User.name.expression
         )
         assert User.y.property.columns[0].element._raw_columns[1] is users.c.id