From: Mike Bayer Date: Thu, 9 Feb 2017 02:05:16 +0000 (-0500) Subject: Check for columns not part of mapping, correct mapping for eager_defaults X-Git-Tag: rel_1_1_6~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=540bcff90d3e9234a776790e3fdeb8c9b8a8b443;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 d4b95e6ee1..7a462eb086 100644 --- 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 4d1e38d3f6..ad268c1273 100644 --- 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 0772e66b5b..26cf9fa01a 100644 --- 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="*")