diff options
author | Mike Bayer <mike_mp@zzzcomputing.com> | 2014-03-28 18:00:35 -0400 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2014-03-28 18:00:35 -0400 |
commit | d6618e41195587291c4a82af0db43fbd993b6c77 (patch) | |
tree | a93e608f2a145621ef19dfc869b78806e9b2f5bd | |
parent | a0329f71ad48b16ad9dad44d5a5cb2c920e569f1 (diff) | |
download | sqlalchemy-d6618e41195587291c4a82af0db43fbd993b6c77.tar.gz |
- Added new parameter :paramref:`.mapper.confirm_deleted_rows`. Defaults
to True, indicates that a series of DELETE statements should confirm
that the cursor rowcount matches the number of primary keys that should
have matched; this behavior had been taken off in most cases
(except when version_id is used) to support the unusual edge case of
self-referential ON DELETE CASCADE; to accomodate this, the message
is now just a warning, not an exception, and the flag can be used
to indicate a mapping that expects self-refererntial cascaded
deletes of this nature. See also :ticket:`2403` for background on the
original change. re: #2403 fix #3007
-rw-r--r-- | doc/build/changelog/changelog_09.rst | 15 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 18 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/persistence.py | 39 | ||||
-rw-r--r-- | test/orm/test_unitofwork.py | 1 | ||||
-rw-r--r-- | test/orm/test_unitofworkv2.py | 36 |
5 files changed, 80 insertions, 29 deletions
diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index d0116b333..35078e53b 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -15,6 +15,21 @@ :version: 0.9.4 .. change:: + :tags: feature, orm + :tickets: 3007 + + Added new parameter :paramref:`.mapper.confirm_deleted_rows`. Defaults + to True, indicates that a series of DELETE statements should confirm + that the cursor rowcount matches the number of primary keys that should + have matched; this behavior had been taken off in most cases + (except when version_id is used) to support the unusual edge case of + self-referential ON DELETE CASCADE; to accomodate this, the message + is now just a warning, not an exception, and the flag can be used + to indicate a mapping that expects self-refererntial cascaded + deletes of this nature. See also :ticket:`2403` for background on the + original change. + + .. change:: :tags: bug, ext, automap :tickets: 3004 diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 4747c075d..a939cb9c7 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -110,6 +110,7 @@ class Mapper(_InspectionAttr): include_properties=None, exclude_properties=None, passive_updates=True, + confirm_deleted_rows=True, eager_defaults=False, legacy_is_orphan=False, _compiled_cache_size=100, @@ -208,6 +209,17 @@ class Mapper(_InspectionAttr): See the section :ref:`concrete_inheritance` for an example. + :param confirm_deleted_rows: defaults to True; when a DELETE occurs + of one more more rows based on specific primary keys, a warning is + emitted when the number of rows matched does not equal the number + of rows expected. This parameter may be set to False to handle the case + where database ON DELETE CASCADE rules may be deleting some of those + rows automatically. The warning may be changed to an exception + in a future release. + + .. versionadded:: 0.9.4 - added :paramref:`.mapper.confirm_deleted_rows` + as well as conditional matched row checking on delete. + :param eager_defaults: if True, the ORM will immediately fetch the value of server-generated default values after an INSERT or UPDATE, rather than leaving them as expired to be fetched on next access. @@ -545,9 +557,13 @@ class Mapper(_InspectionAttr): self._compiled_cache_size = _compiled_cache_size self._reconstructor = None self._deprecated_extensions = util.to_list(extension or []) - self.allow_partial_pks = allow_partial_pks + if self.inherits and not self.concrete: + self.confirm_deleted_rows = False + else: + self.confirm_deleted_rows = confirm_deleted_rows + self._set_with_polymorphic(with_polymorphic) if isinstance(self.local_table, expression.SelectBase): diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py index d62e803ee..1bd432f15 100644 --- a/lib/sqlalchemy/orm/persistence.py +++ b/lib/sqlalchemy/orm/persistence.py @@ -681,19 +681,14 @@ def _emit_delete_statements(base_mapper, uowtransaction, cached_connections, expected = len(del_objects) rows_matched = -1 + only_warn = False if connection.dialect.supports_sane_multi_rowcount: c = connection.execute(statement, del_objects) - # only do a row check if we have versioning turned on. - # unfortunately, we *cannot* do a check on the number of - # rows matched here in general, as there is the edge case - # of a table that has a self-referential foreign key with - # ON DELETE CASCADE on it, see #2403. I'm not sure how we can - # resolve this, unless we require special configuration - # to enable "count rows" for certain mappings, or to disable - # it, or to based on it relationship(), not sure. - if need_version_id: - rows_matched = c.rowcount + if not need_version_id: + only_warn = True + + rows_matched = c.rowcount elif need_version_id: if connection.dialect.supports_sane_rowcount: @@ -713,12 +708,24 @@ def _emit_delete_statements(base_mapper, uowtransaction, cached_connections, else: connection.execute(statement, del_objects) - if rows_matched > -1 and expected != rows_matched: - raise orm_exc.StaleDataError( - "DELETE statement on table '%s' expected to " - "delete %d row(s); %d were matched." % - (table.description, expected, rows_matched) - ) + if base_mapper.confirm_deleted_rows and \ + rows_matched > -1 and expected != rows_matched: + if only_warn: + util.warn( + "DELETE statement on table '%s' expected to " + "delete %d row(s); %d were matched. Please set " + "confirm_deleted_rows=False within the mapper " + "configuration to prevent this warning." % + (table.description, expected, rows_matched) + ) + else: + raise orm_exc.StaleDataError( + "DELETE statement on table '%s' expected to " + "delete %d row(s); %d were matched. Please set " + "confirm_deleted_rows=False within the mapper " + "configuration to prevent this warning." % + (table.description, expected, rows_matched) + ) def _finalize_insert_update_commands(base_mapper, uowtransaction, states_to_insert, states_to_update): diff --git a/test/orm/test_unitofwork.py b/test/orm/test_unitofwork.py index 61f736ce6..4776e2988 100644 --- a/test/orm/test_unitofwork.py +++ b/test/orm/test_unitofwork.py @@ -562,6 +562,7 @@ class BatchDeleteIgnoresRowcountTest(fixtures.DeclarativeMappedTest): class A(cls.DeclarativeBasic): __tablename__ = 'A' __table_args__ = dict(test_needs_fk=True) + __mapper_args__ = {"confirm_deleted_rows": False} id = Column(Integer, primary_key=True) parent_id = Column(Integer, ForeignKey('A.id', ondelete='CASCADE')) diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py index 0ac3349af..5512535e5 100644 --- a/test/orm/test_unitofworkv2.py +++ b/test/orm/test_unitofworkv2.py @@ -3,6 +3,7 @@ from sqlalchemy import testing from sqlalchemy.testing import engines from sqlalchemy.testing.schema import Table, Column from test.orm import _fixtures +from sqlalchemy import exc from sqlalchemy.testing import fixtures from sqlalchemy import Integer, String, ForeignKey, func from sqlalchemy.orm import mapper, relationship, backref, \ @@ -1284,7 +1285,7 @@ class BasicStaleChecksTest(fixtures.MappedTest): Column('data', Integer) ) - def _fixture(self): + def _fixture(self, confirm_deleted_rows=True): parent, child = self.tables.parent, self.tables.child class Parent(fixtures.BasicEntity): @@ -1295,8 +1296,8 @@ class BasicStaleChecksTest(fixtures.MappedTest): mapper(Parent, parent, properties={ 'child':relationship(Child, uselist=False, cascade="all, delete-orphan", - backref="parent") - }) + backref="parent"), + }, confirm_deleted_rows=confirm_deleted_rows) mapper(Child, child) return Parent, Child @@ -1318,7 +1319,7 @@ class BasicStaleChecksTest(fixtures.MappedTest): ) @testing.requires.sane_multi_rowcount - def test_delete_multi_missing(self): + def test_delete_multi_missing_warning(self): Parent, Child = self._fixture() sess = Session() p1 = Parent(id=1, data=2, child=None) @@ -1330,16 +1331,27 @@ class BasicStaleChecksTest(fixtures.MappedTest): sess.delete(p1) sess.delete(p2) + assert_raises_message( + exc.SAWarning, + "DELETE statement on table 'parent' expected to " + "delete 2 row\(s\); 0 were matched.", + sess.flush + ) + + @testing.requires.sane_multi_rowcount + def test_delete_multi_missing_allow(self): + Parent, Child = self._fixture(confirm_deleted_rows=False) + sess = Session() + p1 = Parent(id=1, data=2, child=None) + p2 = Parent(id=2, data=3, child=None) + sess.add_all([p1, p2]) sess.flush() - # see issue #2403 - we *cannot* use rowcount here, as - # self-referential DELETE CASCADE could have deleted rows - #assert_raises_message( - # orm_exc.StaleDataError, - # "DELETE statement on table 'parent' expected to " - # "delete 2 row\(s\); 0 were matched.", - # sess.flush - #) + sess.execute(self.tables.parent.delete()) + sess.delete(p1) + sess.delete(p2) + + sess.flush() class BatchInsertsTest(fixtures.MappedTest, testing.AssertsExecutionResults): |