From: Mike Bayer Date: Thu, 25 Jun 2015 01:53:15 +0000 (-0400) Subject: - Fixed a major regression in the 1.0 series where the version_id_counter X-Git-Tag: rel_1_0_6~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1e2f1f5baabd4ec8866ec19b586bb493ea099852;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Fixed a major regression in the 1.0 series where the version_id_counter feature would cause an object's version counter to be incremented when there was no net change to the object's row, but instead an object related to it via relationship (e.g. typically many-to-one) were associated or de-associated with it, resulting in an UPDATE statement that updates the object's version counter and nothing else. In the use case where the relatively recent "server side" and/or "programmatic/conditional" version counter feature were used (e.g. setting version_id_generator to False), the bug could cause an UPDATE without a valid SET clause to be emitted. fixes #3465 --- diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index ccc92e9373..c588ed7531 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -18,6 +18,21 @@ .. changelog:: :version: 1.0.6 + .. change:: + :tags: bug, orm + :tickets: 3465 + + Fixed a major regression in the 1.0 series where the version_id_counter + feature would cause an object's version counter to be incremented + when there was no net change to the object's row, but instead an object + related to it via relationship (e.g. typically many-to-one) + were associated or de-associated with it, resulting in an UPDATE + statement that updates the object's version counter and nothing else. + In the use case where the relatively recent "server side" and/or + "programmatic/conditional" version counter feature were used + (e.g. setting version_id_generator to False), the bug could cause an + UPDATE without a valid SET clause to be emitted. + .. change:: :tags: bug, mssql :tickets: 3464 diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py index a42ed2f7c2..4f074df8e6 100644 --- a/lib/sqlalchemy/orm/persistence.py +++ b/lib/sqlalchemy/orm/persistence.py @@ -461,6 +461,23 @@ def _collect_update_commands( if update_version_id is not None and \ mapper.version_id_col in mapper._cols_by_table[table]: + + if not bulk and not (params or value_params): + # HACK: check for history in other tables, in case the + # history is only in a different table than the one + # where the version_id_col is. This logic was lost + # from 0.9 -> 1.0.0 and restored in 1.0.6. + for prop in mapper._columntoproperty.values(): + history = ( + state.manager[prop.key].impl.get_history( + state, state_dict, + attributes.PASSIVE_NO_INITIALIZE)) + if history.added: + break + else: + # no net change, break + continue + col = mapper.version_id_col params[col._label] = update_version_id @@ -469,7 +486,7 @@ def _collect_update_commands( val = mapper.version_id_generator(update_version_id) params[col.key] = val - if not (params or value_params): + elif not (params or value_params): continue if bulk: diff --git a/test/orm/test_versioning.py b/test/orm/test_versioning.py index 8348cb5883..2d447682a6 100644 --- a/test/orm/test_versioning.py +++ b/test/orm/test_versioning.py @@ -355,6 +355,93 @@ class VersioningTest(fixtures.MappedTest): ) +class NoBumpOnRelationshipTest(fixtures.MappedTest): + __backend__ = True + + @classmethod + def define_tables(cls, metadata): + Table( + 'a', metadata, + Column('id', Integer, primary_key=True), + Column('version_id', Integer) + ) + Table( + 'b', metadata, + Column('id', Integer, primary_key=True), + Column('a_id', ForeignKey('a.id')) + ) + + @classmethod + def setup_classes(cls): + class A(cls.Basic): + pass + + class B(cls.Basic): + pass + + def _run_test(self, auto_version_counter=True): + A, B = self.classes('A', 'B') + s = Session() + if auto_version_counter: + a1 = A() + else: + a1 = A(version_id=1) + s.add(a1) + s.commit() + eq_(a1.version_id, 1) + + b1 = B() + b1.a = a1 + s.add(b1) + s.commit() + + eq_(a1.version_id, 1) + + def test_plain_counter(self): + A, B = self.classes('A', 'B') + a, b = self.tables('a', 'b') + + mapper( + A, a, properties={ + 'bs': relationship(B, backref='a') + }, + version_id_col=a.c.version_id, + ) + mapper(B, b) + + self._run_test() + + def test_functional_counter(self): + A, B = self.classes('A', 'B') + a, b = self.tables('a', 'b') + + mapper( + A, a, properties={ + 'bs': relationship(B, backref='a') + }, + version_id_col=a.c.version_id, + version_id_generator=lambda num: (num or 0) + 1 + ) + mapper(B, b) + + self._run_test() + + def test_no_counter(self): + A, B = self.classes('A', 'B') + a, b = self.tables('a', 'b') + + mapper( + A, a, properties={ + 'bs': relationship(B, backref='a') + }, + version_id_col=a.c.version_id, + version_id_generator=False + ) + mapper(B, b) + + self._run_test(False) + + class ColumnTypeTest(fixtures.MappedTest): __backend__ = True @@ -587,6 +674,53 @@ class AlternateGeneratorTest(fixtures.MappedTest): sess2.commit +class PlainInheritanceTest(fixtures.MappedTest): + __backend__ = True + + @classmethod + def define_tables(cls, metadata): + Table( + 'base', metadata, + Column( + 'id', Integer, primary_key=True, + test_needs_autoincrement=True), + Column('version_id', Integer, nullable=True), + Column('data', String(50)) + ) + Table( + 'sub', metadata, + Column('id', Integer, ForeignKey('base.id'), primary_key=True), + Column('sub_data', String(50)) + ) + + @classmethod + def setup_classes(cls): + + class Base(cls.Basic): + pass + + class Sub(Base): + pass + + def test_update_child_table_only(self): + Base, sub, base, Sub = ( + self.classes.Base, self.tables.sub, self.tables.base, + self.classes.Sub) + + mapper(Base, base, version_id_col=base.c.version_id) + mapper(Sub, sub, inherits=Base) + + s = Session() + s1 = Sub(data='b', sub_data='s') + s.add(s1) + s.commit() + + s1.sub_data = 's2' + s.commit() + + eq_(s1.version_id, 2) + + class InheritanceTwoVersionIdsTest(fixtures.MappedTest): """Test versioning where both parent/child table have a versioning column.