- 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 ccc92e9373dd4faf6a12d7d5b3795c1eb1e93c37..c588ed7531a53f39588e5949548c4bc912e29026 100644 (file)
--- 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 a42ed2f7c2f052fa1871a1bd9025ffc4e05c0d94..4f074df8e69cdbb436ee605e0e66b572f88c1743 100644 (file)
--- 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 8348cb58832645953c4ff2addf6577aacbe2fb52..2d447682a61af0d23dcd11bef524c6e461475af3 100644 (file)
--- 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.