Filter attributes we don't map during a load_scalar_attributes

Fixed bug where a descriptor that is elsewhere a mapped column
or relationship within a hierarchy based on :class:`.AbstractConcreteBase`
would be referred towards during a refresh operation, causing an error
as the attribute is not mapped as a mapper property.
A similar issue can arise for other attributes like the "type" column
added by :class:`.AbstractConcreteBase` if the class fails to include
"concrete=True" in its mapper, however the check here should also
prevent that scenario from causing a problem.

Change-Id: I407b07a3a3e2c374da19fc86ed44b987d595dcfa
Fixes: #4124
(cherry picked from commit fd4289c5829d6498495ac59fe1dccb23b4975281)

diff --git a/doc/build/changelog/unreleased_11/4124.rst b/doc/build/changelog/unreleased_11/4124.rst
new file mode 100644 (file)
index 0000000..27ad67b
--- /dev/null
+++ b/doc/build/changelog/unreleased_11/4124.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, orm, declarative
+    :tickets: 4124
+    :versions: 1.2.0b4
+
+    Fixed bug where a descriptor that is elsewhere a mapped column
+    or relationship within a hierarchy based on :class:`.AbstractConcreteBase`
+    would be referred towards during a refresh operation, causing an error
+    as the attribute is not mapped as a mapper property.
+    A similar issue can arise for other attributes like the "type" column
+    added by :class:`.AbstractConcreteBase` if the class fails to include
+    "concrete=True" in its mapper, however the check here should also
+    prevent that scenario from causing a problem.

diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index 6f0c860cc5144f29ef453bf981f54056138410fb..fe9f9e21a8fcc63b0b71c2419890f81380b4c6b8 100644 (file)
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -662,6 +662,15 @@ def load_scalar_attributes(mapper, state, attribute_names):
 
     result = False
 
+    # in the case of inheritance, particularly concrete and abstract
+    # concrete inheritance, the class manager might have some keys
+    # of attributes on the superclass that we didn't actually map.
+    # These could be mapped as "concrete, dont load" or could be completely
+    # exluded from the mapping and we know nothing about them.  Filter them
+    # here to prevent them from coming through.
+    if attribute_names:
+        attribute_names = attribute_names.intersection(mapper.attrs.keys())
+
     if mapper.inherits and not mapper.concrete:
         # because we are using Core to produce a select() that we
         # pass to the Query, we aren't calling setup() for mapped

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 83f8eca1ce6b7d1585071724861d88b686076615..14debb0c831fe9d1cb5a4e07afc5b09065ee264a 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1098,6 +1098,7 @@ class Mapper(InspectionAttr):
         self.inherits._inheriting_mappers.append(self)
         self.passive_updates = self.inherits.passive_updates
         self._all_tables = self.inherits._all_tables
+
         for key, prop in mapper._props.items():
             if key not in self._props and \
                 not self._should_exclude(key, key, local=False,

diff --git a/test/ext/declarative/test_inheritance.py b/test/ext/declarative/test_inheritance.py
index e57320cfe4433494f035c8b781882222e00473bf..7ced8ec157bb672b99c04c4a62a63167554f1e1b 100644 (file)
--- a/test/ext/declarative/test_inheritance.py
+++ b/test/ext/declarative/test_inheritance.py
@@ -1092,6 +1092,10 @@ class ConcreteInhTest(_RemoveListeners, DeclarativeTestBase):
             eq_(sess.query(Manager).all(), [Manager(name='dogbert')])
             eq_(sess.query(Boss).all(), [Boss(name='pointy haired')])
 
+        e1 = sess.query(Engineer).order_by(Engineer.name).first()
+        sess.expire(e1)
+        eq_(e1.name, 'dilbert')
+
     def test_explicit(self):
         engineers = Table(
             'engineers', Base.metadata,
@@ -1213,6 +1217,55 @@ class ConcreteInhTest(_RemoveListeners, DeclarativeTestBase):
 
         self._roundtrip(Employee, Manager, Engineer, Boss)
 
+    def test_abstract_concrete_extension_descriptor_refresh(self):
+        class Employee(AbstractConcreteBase, Base, fixtures.ComparableEntity):
+            @declared_attr
+            def name(cls):
+                return Column(String(50))
+
+        class Manager(Employee):
+            __tablename__ = 'manager'
+            employee_id = Column(Integer, primary_key=True,
+                                 test_needs_autoincrement=True)
+            paperwork = Column(String(10))
+            __mapper_args__ = {
+                'polymorphic_identity': 'manager', 'concrete': True}
+
+        class Engineer(Employee):
+            __tablename__ = 'engineer'
+            employee_id = Column(Integer, primary_key=True,
+                                 test_needs_autoincrement=True)
+
+            @property
+            def paperwork(self):
+                return "p"
+
+            __mapper_args__ = {
+                'polymorphic_identity': 'engineer', 'concrete': True}
+
+        Base.metadata.create_all()
+        sess = Session()
+        sess.add(Engineer(name='d'))
+        sess.commit()
+
+        # paperwork is excluded because there's a descritor; so it is
+        # not in the Engineers mapped properties at all, though is inside the
+        # class manager.   Maybe it shouldn't be in the class manager either.
+        assert 'paperwork' in Engineer.__mapper__.class_manager
+        assert 'paperwork' not in Engineer.__mapper__.attrs.keys()
+
+        # type currently does get mapped, as a
+        # ConcreteInheritedProperty, which means, "ignore this thing inherited
+        # from the concrete base".   if we didn't specify concrete=True, then
+        # this one gets stuck in the error condition also.
+        assert 'type' in Engineer.__mapper__.class_manager
+        assert 'type' in Engineer.__mapper__.attrs.keys()
+
+        e1 = sess.query(Engineer).first()
+        eq_(e1.name, 'd')
+        sess.expire(e1)
+        eq_(e1.name, 'd')
+
     def test_concrete_extension(self):
         class Employee(ConcreteBase, Base, fixtures.ComparableEntity):
             __tablename__ = 'employee'