summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMike Bayer <mike_mp@zzzcomputing.com>2022-06-28 18:55:19 -0400
committerMike Bayer <mike_mp@zzzcomputing.com>2022-06-29 11:22:59 -0400
commit9d12d493eb38f958c2d50da28f83ccc6de01f0dc (patch)
tree0063d1e4d2be517d3509d50c1b3f64201fc0ccce
parentf19e50ab75cfc904acef31434c92542f3ab50d61 (diff)
downloadsqlalchemy-9d12d493eb38f958c2d50da28f83ccc6de01f0dc.tar.gz
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
-rw-r--r--doc/build/changelog/unreleased_14/8190.rst8
-rw-r--r--lib/sqlalchemy/orm/decl_base.py41
-rw-r--r--test/orm/declarative/test_mixin.py47
3 files changed, 92 insertions, 4 deletions
diff --git a/doc/build/changelog/unreleased_14/8190.rst b/doc/build/changelog/unreleased_14/8190.rst
new file mode 100644
index 000000000..934e44cf5
--- /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 1366bedf2..62251fa2b 100644
--- a/lib/sqlalchemy/orm/decl_base.py
+++ b/lib/sqlalchemy/orm/decl_base.py
@@ -714,7 +714,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
@@ -727,10 +734,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(
+ 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.
+ collected_attributes.update(locally_collected_columns)
for (
name,
@@ -738,6 +769,7 @@ class _ClassScanMapperConfig(_MapperConfig):
annotation,
is_dataclass_field,
) in local_attributes_for_class():
+
if re.match(r"^__.+__$", name):
if name == "__mapper_args__":
check_decl = _check_declared_props_nocascade(
@@ -1096,10 +1128,10 @@ class _ClassScanMapperConfig(_MapperConfig):
[], Iterable[Tuple[str, Any, Any, bool]]
],
attribute_is_overridden: Callable[[str, Any], bool],
- ) -> None:
+ ) -> Dict[str, Union[Column[Any], MappedColumn[Any]]]:
cls = self.cls
dict_ = self.clsdict_view
- collected_attributes = self.collected_attributes
+ locally_collected_attributes = {}
column_copies = self.column_copies
# copy mixin columns to the mapped class
@@ -1132,9 +1164,10 @@ class _ClassScanMapperConfig(_MapperConfig):
)
column_copies[obj] = copy_ = obj._copy()
- collected_attributes[name] = copy_
+ locally_collected_attributes[name] = copy_
setattr(cls, name, copy_)
+ return locally_collected_attributes
def _extract_mappable_attributes(self) -> None:
cls = self.cls
diff --git a/test/orm/declarative/test_mixin.py b/test/orm/declarative/test_mixin.py
index 36840b2d7..d509b6e99 100644
--- a/test/orm/declarative/test_mixin.py
+++ b/test/orm/declarative/test_mixin.py
@@ -2216,6 +2216,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".id AS user_id, "user".status AS user_status '
+ '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):