produce column copies up the whole hierarchy first

Fixed issue where a hierarchy of classes set up as an abstract or mixin
declarative classes could not declare standalone columns on a superclass
that would then be copied correctly to a :class:`_orm.declared_attr`
callable that wanted to make use of them on a descendant class.

Originally it looked like this would produce an ordering change,
however an adjustment to the flow for produce_column_copies
has avoided that for now.

Fixes: #8190
Change-Id: I4e2ee74edb110793eb42691c3e4a0e0535fba7e9
(cherry picked from commit 9d12d493eb38f958c2d50da28f83ccc6de01f0dc)

diff --git a/doc/build/changelog/unreleased_14/8190.rst b/doc/build/changelog/unreleased_14/8190.rst
new file mode 100644 (file)
index 0000000..934e44c
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/8190.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, orm, declarative
+    :tickets: 8190
+
+    Fixed issue where a hierarchy of classes set up as an abstract or mixin
+    declarative classes could not declare standalone columns on a superclass
+    that would then be copied correctly to a :class:`_orm.declared_attr`
+    callable that wanted to make use of them on a descendant class.

diff --git a/lib/sqlalchemy/orm/decl_base.py b/lib/sqlalchemy/orm/decl_base.py
index ed4ccd19682f341bde760d74799ec71e83763766..6e1c79745fa8ec95efb155267ef886d23c68c3aa 100644 (file)
--- a/lib/sqlalchemy/orm/decl_base.py
+++ b/lib/sqlalchemy/orm/decl_base.py
@@ -459,7 +459,14 @@ class _ClassScanMapperConfig(_MapperConfig):
 
         attribute_is_overridden = self._cls_attr_override_checker(self.cls)
 
+        bases = []
+
         for base in cls.__mro__:
+            # collect bases and make sure standalone columns are copied
+            # to be the column they will ultimately be on the class,
+            # so that declared_attr functions use the right columns.
+            # need to do this all the way up the hierarchy first
+            # (see #8190)
 
             class_mapped = (
                 base is not cls
@@ -472,9 +479,34 @@ class _ClassScanMapperConfig(_MapperConfig):
             local_attributes_for_class = self._cls_attr_resolver(base)
 
             if not class_mapped and base is not cls:
-                self._produce_column_copies(
-                    local_attributes_for_class, attribute_is_overridden
+                locally_collected_columns = self._produce_column_copies(
+                    local_attributes_for_class,
+                    attribute_is_overridden,
+                )
+            else:
+                locally_collected_columns = {}
+
+            bases.append(
+                (
+                    base,
+                    class_mapped,
+                    local_attributes_for_class,
+                    locally_collected_columns,
                 )
+            )
+
+        for (
+            base,
+            class_mapped,
+            local_attributes_for_class,
+            locally_collected_columns,
+        ) in bases:
+
+            # this transfer can also take place as we scan each name
+            # for finer-grained control of how collected_attributes is
+            # populated, as this is what impacts column ordering.
+            # however it's simpler to get it out of the way here.
+            dict_.update(locally_collected_columns)
 
             for name, obj, is_dataclass in local_attributes_for_class():
                 if name == "__mapper_args__":
@@ -640,6 +672,7 @@ class _ClassScanMapperConfig(_MapperConfig):
     ):
         cls = self.cls
         dict_ = self.dict_
+        locally_collected_attributes = {}
         column_copies = self.column_copies
         # copy mixin columns to the mapped class
 
@@ -664,7 +697,8 @@ class _ClassScanMapperConfig(_MapperConfig):
                     column_copies[obj] = copy_ = obj._copy()
                     copy_._creation_order = obj._creation_order
                     setattr(cls, name, copy_)
-                    dict_[name] = copy_
+                    locally_collected_attributes[name] = copy_
+        return locally_collected_attributes
 
     def _extract_mappable_attributes(self):
         cls = self.cls

diff --git a/test/orm/declarative/test_mixin.py b/test/orm/declarative/test_mixin.py
index 78ab4dbfc3e12b830b27fd5d4127025bfeae8396..f8e0cf5adb34865ced816a635eca2f7c88e19f4e 100644 (file)
--- a/test/orm/declarative/test_mixin.py
+++ b/test/orm/declarative/test_mixin.py
@@ -2140,6 +2140,53 @@ class DeclaredAttrTest(DeclarativeTestBase, testing.AssertsCompiledSQL):
             "> :param_1",
         )
 
+    def test_multilevel_mixin_attr_refers_to_column_copies(self):
+        """test #8190.
+
+        This test is the same idea as test_mixin_attr_refers_to_column_copies
+        but tests the column copies from superclasses.
+
+        """
+        counter = mock.Mock()
+
+        class SomeOtherMixin:
+            status = Column(String)
+
+        class HasAddressCount(SomeOtherMixin):
+            id = Column(Integer, primary_key=True)
+
+            @declared_attr
+            def address_count(cls):
+                counter(cls.id)
+                counter(cls.status)
+                return column_property(
+                    select(func.count(Address.id))
+                    .where(Address.user_id == cls.id)
+                    .where(cls.status == "some status")
+                    .scalar_subquery()
+                )
+
+        class Address(Base):
+            __tablename__ = "address"
+            id = Column(Integer, primary_key=True)
+            user_id = Column(ForeignKey("user.id"))
+
+        class User(Base, HasAddressCount):
+            __tablename__ = "user"
+
+        eq_(counter.mock_calls, [mock.call(User.id), mock.call(User.status)])
+
+        sess = fixture_session()
+        self.assert_compile(
+            sess.query(User).having(User.address_count > 5),
+            "SELECT (SELECT count(address.id) AS count_1 FROM address "
+            'WHERE address.user_id = "user".id AND "user".status = :param_1) '
+            'AS anon_1, "user".status AS user_status, "user".id AS user_id '
+            'FROM "user" HAVING (SELECT count(address.id) AS count_1 '
+            'FROM address WHERE address.user_id = "user".id '
+            'AND "user".status = :param_1) > :param_2',
+        )
+
 
 class AbstractTest(DeclarativeTestBase):
     def test_abstract_boolean(self):