Check for columns not part of mapping, correct mapping for eager_defaults

Fixed two closely related bugs involving the mapper eager_defaults
flag in conjunction with single-table inheritance; one where the
eager defaults logic would inadvertently try to access a column
that's part of the mapper's "exclude_properties" list (used by
Declarative with single table inheritance) during the eager defaults
fetch, and the other where the full load of the row in order to
fetch the defaults would fail to use the correct inheriting mapper.

Fixes: #3908
Change-Id: Ie745174c917d512e2c46d9e3cc14512cde53cc9a

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index d4b95e6ee124510feb4850dbba17600ad3c391af..7a462eb086e84b391d6197246e2eeda0014d104b 100644 (file)
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -34,6 +34,19 @@
         RETURNING that eager_defaults tries to use, they should not be
         post-SELECTed either.
 
+    .. change:: 3908
+        :tags: bug, orm
+        :tickets: 3908
+
+        Fixed two closely related bugs involving the mapper eager_defaults
+        flag in conjunction with single-table inheritance; one where the
+        eager defaults logic would inadvertently try to access a column
+        that's part of the mapper's "exclude_properties" list (used by
+        Declarative with single table inheritance) during the eager defaults
+        fetch, and the other where the full load of the row in order to
+        fetch the defaults would fail to use the correct inheriting mapper.
+
+
     .. change:: 3905
         :tags: bug, sql
         :tickets: 3905

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 4d1e38d3f6ba593805b7fa7959c0a2868b60dd10..ad268c127330e27015df2702875668ca4378449c 100644 (file)
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -1007,7 +1007,7 @@ def _finalize_insert_update_commands(base_mapper, uowtransaction, states):
         if toload_now:
             state.key = base_mapper._identity_key_from_state(state)
             loading.load_on_ident(
-                uowtransaction.session.query(base_mapper),
+                uowtransaction.session.query(mapper),
                 state.key, refresh_state=state,
                 only_load_props=toload_now)
 
@@ -1044,9 +1044,16 @@ def _postfetch(mapper, uowtransaction, table,
                 # distinctly, don't step on the values here
                 if col.primary_key and result.context.isinsert:
                     continue
-                dict_[mapper._columntoproperty[col].key] = row[col]
-                if refresh_flush:
-                    load_evt_attrs.append(mapper._columntoproperty[col].key)
+
+                # note that columns can be in the "return defaults" that are
+                # not mapped to this mapper, typically because they are
+                # "excluded", which can be specified directly or also occurs
+                # when using declarative w/ single table inheritance
+                prop = mapper._columntoproperty.get(col)
+                if prop:
+                    dict_[prop.key] = row[col]
+                    if refresh_flush:
+                        load_evt_attrs.append(prop.key)
 
     for c in prefetch_cols:
         if c.key in params and c in mapper._columntoproperty:

diff --git a/test/orm/inheritance/test_single.py b/test/orm/inheritance/test_single.py
index 0772e66b5bbf41b3183c87209fa31183c9dae9b9..26cf9fa01a0ebbbb66508179c47a497ed3356cb4 100644 (file)
--- a/test/orm/inheritance/test_single.py
+++ b/test/orm/inheritance/test_single.py
@@ -6,7 +6,7 @@ from sqlalchemy import testing
 from test.orm import _fixtures
 from sqlalchemy.testing import fixtures, AssertsCompiledSQL
 from sqlalchemy.testing.schema import Table, Column
-
+from sqlalchemy import inspect
 
 class SingleInheritanceTest(testing.AssertsCompiledSQL, fixtures.MappedTest):
     __dialect__ = 'default'
@@ -1086,3 +1086,75 @@ class SingleOnJoinedTest(fixtures.MappedTest):
                  Manager(
                      name='m1', employee_data='ed2', manager_data='md1')])
         self.assert_sql_count(testing.db, go, 1)
+
+
+class EagerDefaultEvalTest(fixtures.DeclarativeMappedTest):
+    @classmethod
+    def setup_classes(cls, with_polymorphic=None, include_sub_defaults=False):
+        Base = cls.DeclarativeBasic
+
+        class Foo(Base):
+            __tablename__ = "foo"
+            id = Column(
+                Integer, primary_key=True, test_needs_autoincrement=True)
+            type = Column(String(50))
+            created_at = Column(Integer, server_default="5")
+
+            __mapper_args__ = {
+                "polymorphic_on": type,
+                "polymorphic_identity": "foo",
+                "eager_defaults": True,
+                "with_polymorphic": with_polymorphic
+            }
+
+        class Bar(Foo):
+            bar = Column(String(50))
+            if include_sub_defaults:
+                bat = Column(Integer, server_default="10")
+
+            __mapper_args__ = {
+                "polymorphic_identity": "bar",
+            }
+
+    def test_persist_foo(self):
+        Foo = self.classes.Foo
+
+        foo = Foo()
+
+        session = Session()
+        session.add(foo)
+        session.flush()
+
+        eq_(foo.__dict__['created_at'], 5)
+
+        assert 'bat' not in foo.__dict__
+
+        session.close()
+
+    def test_persist_bar(self):
+        Bar = self.classes.Bar
+        bar = Bar()
+        session = Session()
+        session.add(bar)
+        session.flush()
+
+        eq_(bar.__dict__['created_at'], 5)
+
+        if 'bat' in inspect(Bar).attrs:
+            eq_(bar.__dict__['bat'], 10)
+
+        session.close()
+
+
+class EagerDefaultEvalTestSubDefaults(EagerDefaultEvalTest):
+    @classmethod
+    def setup_classes(cls):
+        super(EagerDefaultEvalTestSubDefaults, cls).setup_classes(
+            include_sub_defaults=True)
+
+
+class EagerDefaultEvalTestPolymorphic(EagerDefaultEvalTest):
+    @classmethod
+    def setup_classes(cls):
+        super(EagerDefaultEvalTestPolymorphic, cls).setup_classes(
+            with_polymorphic="*")