From: G Allajmi Date: Mon, 8 Dec 2025 13:03:49 +0000 (-0500) Subject: Fix adding property to mapper before mapping is complete X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e4a802f99a0bca813dd29e8dc88bb5e5aaddff93;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Fix adding property to mapper before mapping is complete Fixed issue where calling :meth:`.Mapper.add_property` within mapper event hooks such as :meth:`.MapperEvents.instrument_class`, :meth:`.MapperEvents.after_mapper_constructed`, or :meth:`.MapperEvents.before_mapper_configured` would raise an ``AttributeError`` because the mapper's internal property collections were not yet initialized. The :meth:`.Mapper.add_property` method now handles early-stage property additions correctly, allowing properties including column properties, deferred columns, and relationships to be added during mapper initialization events. Pull request courtesy G Allajmi. Fixes: #12858 Closes: #13023 Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/13023 Pull-request-sha: fa9a0ae4bb07819307e6c9d6ec5fd4c9706bf67e Change-Id: Ibb794c401102a8d23d157b40353f272d5735b49a --- diff --git a/doc/build/changelog/unreleased_20/12858.rst b/doc/build/changelog/unreleased_20/12858.rst new file mode 100644 index 0000000000..fd0cd30c95 --- /dev/null +++ b/doc/build/changelog/unreleased_20/12858.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: bug, orm + :tickets: 12858 + + Fixed issue where calling :meth:`.Mapper.add_property` within mapper event + hooks such as :meth:`.MapperEvents.instrument_class`, + :meth:`.MapperEvents.after_mapper_constructed`, or + :meth:`.MapperEvents.before_mapper_configured` would raise an + ``AttributeError`` because the mapper's internal property collections were + not yet initialized. The :meth:`.Mapper.add_property` method now handles + early-stage property additions correctly, allowing properties including + column properties, deferred columns, and relationships to be added during + mapper initialization events. Pull request courtesy G Allajmi. diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index fb40c6f10a..370f1a7cc6 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -2037,13 +2037,21 @@ class Mapper( "_configure_property(%s, %s)", key, prop_arg.__class__.__name__ ) + # early setup mode - don't assign any props, only + # ensure a Column is turned into a ColumnProperty. + # see #12858 + early_setup = not hasattr(self, "_props") + if not isinstance(prop_arg, MapperProperty): prop: MapperProperty[Any] = self._property_from_column( - key, prop_arg + key, prop_arg, early_setup ) else: prop = prop_arg + if early_setup: + return prop + if isinstance(prop, properties.ColumnProperty): col = self.persist_selectable.corresponding_column(prop.columns[0]) @@ -2290,16 +2298,17 @@ class Mapper( @util.preload_module("sqlalchemy.orm.descriptor_props") def _property_from_column( - self, - key: str, - column: KeyedColumnElement[Any], + self, key: str, column: KeyedColumnElement[Any], early_setup: bool ) -> ColumnProperty[Any]: """generate/update a :class:`.ColumnProperty` given a :class:`_schema.Column` or other SQL expression object.""" descriptor_props = util.preloaded.orm_descriptor_props - prop = self._props.get(key) + if early_setup: + prop = None + else: + prop = self._props.get(key) if isinstance(prop, properties.ColumnProperty): return self._reconcile_prop_with_incoming_columns( diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index 8bb8bb32c2..a46ee57e2f 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -4,6 +4,7 @@ import re import sqlalchemy as sa from sqlalchemy import column +from sqlalchemy import event from sqlalchemy import ForeignKey from sqlalchemy import func from sqlalchemy import inspect @@ -1031,6 +1032,129 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL): assert hasattr(User, "addresses") assert "addresses" in [p.key for p in m1._polymorphic_properties] + @testing.variation( + "property_type", ["Column", "ColumnProperty", "Relationship"] + ) + @testing.variation( + "event_name", + [ + "instrument_class", + "after_mapper_constructed", + "mapper_configured", + "before_mapper_configured", + ], + ) + def test_add_property_in_mapper_event( + self, property_type, connection, metadata, event_name + ): + """test #12858, ability to add properties of all types within + event hooks where the mapper is not necessarily set up with property + collections. + + """ + + Address = self.classes.Address + + users = Table( + "deferred_users", + metadata, + Column("id", Integer, primary_key=True), + Column("name", String), + ) + + # use an ad-hoc User class. Assigning an event holder to a + # class right now means that class permanently has that mapper + # event associated with it, whenever a new mapper is created. + # if you call event.remove(User), it removes the event from the mapper, + # but not the "event hold" that associates it again on a new mapper. + # this is probably a bug but not the one we came here today to fix + class User: + pass + + def setup_props(mapper, class_): + if property_type.ColumnProperty: + col = Column("new_column", String) + mapper.local_table.append_column(col) + mapper.add_property( + col.key, deferred(col, group="deferred_group") + ) + elif property_type.Column: + col = Column("new_column", String) + mapper.local_table.append_column(col) + mapper.add_property(col.key, col) + elif property_type.Relationship: + mapper.add_property("addresses", relationship(Address)) + else: + property_type.fail() + + event.listen(User, event_name.name, setup_props) + + if property_type.ColumnProperty: + self.mapper( + User, + users, + properties={ + "name": deferred(users.c.name, group="deferred_group") + }, + ) + else: + self.mapper(User, users) + + if property_type.Relationship: + addresses_table = Table( + "deferred_addresses", + metadata, + Column("id", Integer, primary_key=True), + Column("user_id", ForeignKey("deferred_users.id")), + ) + self.mapper(Address, addresses_table) + configure_mappers() + + if property_type.Relationship: + assert hasattr(User, "addresses") + else: + assert hasattr(User, "new_column") + + metadata.create_all(connection) + sess = fixture_session(bind=connection) + + if property_type.Relationship: + u = User( + id=1, + name="testuser", + addresses=[Address()], + ) + else: + u = User(id=1, name="testuser", new_column="test_value") + sess.add(u) + sess.commit() + + sess = Session(connection) + user = sess.query(User).filter_by(id=1).first() + if property_type.ColumnProperty: + assert "id" in user.__dict__ + + # check that the column is deferred + assert "new_column" not in user.__dict__ + assert "name" not in user.__dict__ + + eq_(user.new_column, "test_value") + + assert "new_column" in user.__dict__ + assert "name" in user.__dict__ + eq_(user.name, "testuser") + + elif property_type.Column: + assert "id" in user.__dict__ + assert "new_column" in user.__dict__ + assert "name" in user.__dict__ + eq_(user.new_column, "test_value") + eq_(user.name, "testuser") + elif property_type.Relationship: + eq_(user.addresses[0].user_id, 1) + else: + property_type.fail() + def test_replace_col_prop_w_syn(self): users, User = self.tables.users, self.classes.User