- revert part of c01558ae7f4a for now as we also test that a DELETE of two rows

where one is to be deleted from ON DELETE CASCADE succeeds; the check here makes that fail.
We will need to add an option to enable/disable this check per mapping, will likely
do this in next version

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 2bcaf2bec87522f74926305004c7e440f2cae879..d5e4a1fb6a6541be2ef9e746ef1a2281e0ca509f 100644 (file)
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -18,13 +18,9 @@
 
         Fixed ORM bug where changing the primary key of an object, then marking
         it for DELETE would fail to target the correct row for DELETE.
-        Then to compound matters, basic "number of rows matched" checks were
-        not being performed.  Both issues are fixed, however note that the
-        "rows matched" check requires so-called "sane multi-row count"
-        functionality; the DBAPI's executemany() method must count up the
-        rows matched by individual statements and SQLAlchemy's dialect must
-        mark this feature as supported, currently applies to some mysql dialects,
-        psycopg2, sqlite only.
+        Note that we cannot currently check "number of rows matched" in general
+        for DELETE statements as we can't be sure that a self-referential
+        ON DELETE CASCADE has gotten there first.
 
     .. change::
         :tags: feature, postgresql

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index b4c9c052b4a0cf6402e5e2f55a028981807c3fbc..d62e803ee5fcd0e134221df4ce1aceff1bc4a052 100644 (file)
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -683,7 +683,18 @@ def _emit_delete_statements(base_mapper, uowtransaction, cached_connections,
         rows_matched = -1
         if connection.dialect.supports_sane_multi_rowcount:
             c = connection.execute(statement, del_objects)
-            rows_matched = c.rowcount
+
+            # 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
+
         elif need_version_id:
             if connection.dialect.supports_sane_rowcount:
                 rows_matched = 0

diff --git a/test/orm/test_unitofwork.py b/test/orm/test_unitofwork.py
index e10dfb651ca0199f6e0ff22d5c0016f4f7b721bb..61f736ce60ebd5d9e5d30191541bdc6065bac882 100644 (file)
--- a/test/orm/test_unitofwork.py
+++ b/test/orm/test_unitofwork.py
@@ -569,7 +569,7 @@ class BatchDeleteIgnoresRowcountTest(fixtures.DeclarativeMappedTest):
         A = self.classes.A
         session = Session(testing.db)
 
-        a1, a2 = A(id=1),A(id=2, parent_id=1)
+        a1, a2 = A(id=1), A(id=2, parent_id=1)
 
         session.add_all([a1, a2])
         session.flush()

diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py
index 1dfc0401bb348777a2eb61af8259d1b22304c534..0ac3349afa24a68b0444562262fee29c0b401db1 100644 (file)
--- a/test/orm/test_unitofworkv2.py
+++ b/test/orm/test_unitofworkv2.py
@@ -1330,12 +1330,16 @@ class BasicStaleChecksTest(fixtures.MappedTest):
         sess.delete(p1)
         sess.delete(p2)
 
-        assert_raises_message(
-            orm_exc.StaleDataError,
-            "DELETE statement on table 'parent' expected to "
-                "delete 2 row\(s\); 0 were matched.",
-            sess.flush
-        )
+        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
+        #)
 
 
 class BatchInsertsTest(fixtures.MappedTest, testing.AssertsExecutionResults):