diff options
| -rw-r--r-- | doc/build/changelog/changelog_12.rst | 7 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/persistence.py | 5 | ||||
| -rw-r--r-- | test/orm/test_versioning.py | 73 |
3 files changed, 85 insertions, 0 deletions
diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst index 8cc8cd862..2d1535849 100644 --- a/doc/build/changelog/changelog_12.rst +++ b/doc/build/changelog/changelog_12.rst @@ -26,6 +26,13 @@ and the UPDATE now sets the version_id value to itself, so that concurrency checks still take place. + .. change:: 3673 + :tags: bug, orm + :tickets: 3673 + + The versioning feature does not support NULL for the version counter. + An exception is now raised if the version id is programmatic and + was set to NULL for an UPDATE. Pull request courtesy Diana Clarke. .. change:: 3796 :tags: bug, orm diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py index 86125d6f2..e8a7e4c33 100644 --- a/lib/sqlalchemy/orm/persistence.py +++ b/lib/sqlalchemy/orm/persistence.py @@ -1032,6 +1032,11 @@ def _finalize_insert_update_commands(base_mapper, uowtransaction, states): else: mapper.dispatch.after_update(mapper, connection, state) + if mapper.version_id_col is not None: + if state_dict[mapper._version_id_prop.key] is None: + raise orm_exc.FlushError( + "Instance does not contain a non-NULL version value") + def _postfetch(mapper, uowtransaction, table, state, dict_, result, params, value_params): diff --git a/test/orm/test_versioning.py b/test/orm/test_versioning.py index 4c339d3bb..9ade0a4ff 100644 --- a/test/orm/test_versioning.py +++ b/test/orm/test_versioning.py @@ -20,6 +20,79 @@ def make_uuid(): return uuid.uuid4().hex +class NullVersionIdTest(fixtures.MappedTest): + __backend__ = True + + @classmethod + def define_tables(cls, metadata): + Table('version_table', metadata, + Column('id', Integer, primary_key=True, + test_needs_autoincrement=True), + Column('version_id', Integer), + Column('value', String(40), nullable=False)) + + @classmethod + def setup_classes(cls): + class Foo(cls.Basic): + pass + + def _fixture(self): + Foo, version_table = self.classes.Foo, self.tables.version_table + + mapper( + Foo, version_table, + version_id_col=version_table.c.version_id, + version_id_generator=False, + ) + + s1 = Session() + return s1 + + def test_null_version_id_insert(self): + Foo = self.classes.Foo + + s1 = self._fixture() + f1 = Foo(value='f1') + s1.add(f1) + + # Prior to the fix for #3673, you would have been allowed to insert + # the above record with a NULL version_id and you would have gotten + # the following error when you tried to update it. Now you should + # get a FlushError on the initial insert. + # + # A value is required for bind parameter 'version_table_version_id' + # UPDATE version_table SET value=? + # WHERE version_table.id = ? + # AND version_table.version_id = ? + # parameters: [{'version_table_id': 1, 'value': 'f1rev2'}]] + + assert_raises_message( + sa.orm.exc.FlushError, + "Instance does not contain a non-NULL version value", + s1.commit) + + def test_null_version_id_update(self): + Foo = self.classes.Foo + + s1 = self._fixture() + f1 = Foo(value='f1', version_id=1) + s1.add(f1) + s1.commit() + + # Prior to the fix for #3673, you would have been allowed to update + # the above record with a NULL version_id, and it would look like + # this, post commit: Foo(id=1, value='f1rev2', version_id=None). Now + # you should get a FlushError on update. + + f1.value = 'f1rev2' + f1.version_id = None + + assert_raises_message( + sa.orm.exc.FlushError, + "Instance does not contain a non-NULL version value", + s1.commit) + + class VersioningTest(fixtures.MappedTest): __backend__ = True |