From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 28 Mar 2014 21:42:55 +0000 (-0400)
Subject: - revert part of c01558ae7f4a for now as we also test that a DELETE of two rows
X-Git-Tag: rel_0_8_6~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7473b545749700d092d014224f1642fa21ad193a;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- 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 bcd7ca6c9b..35c1b7f910 100644
--- 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 9ad99d0649..08299f384f 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -659,7 +659,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 7537800575..6eb737a245 100644
--- a/test/orm/test_unitofwork.py
+++ b/test/orm/test_unitofwork.py
@@ -584,7 +584,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 8bdb8cad6b..aaed74dd9b 100644
--- 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):