work around MySQL InnoDB bug, need per-mapper sane cascades flag

[Mike Bayer]

they report the wrong rowcount when you delete two rows that are in a CASCADE relationship:

from sqlalchemy import *
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import Session, relationship

engine = create_engine('mysql://root@localhost/test', echo=True)

# rowcount is not sane as the CASCADE between A/A not counted correctly.
#engine.dialect.supports_sane_rowcount = engine.dialect.supports_sane_multi_rowcount = False

Base = declarative_base(bind=engine)

class A(Base):

    __tablename__ = 'A'
    __table_args__ = {"mysql_engine":"InnoDB"}
    id = Column(Integer, primary_key=True)
    parent_id = Column(Integer, ForeignKey('A.id', ondelete='CASCADE'))

Base.metadata.drop_all()
Base.metadata.create_all()
session = Session(engine)

a1, a2 = A(id=1),A(id=2, parent_id=1)

session.add_all([a1, a2])
session.flush()

session.delete(a1)
session.delete(a2)

session.flush()

we would need to get mapper.delete_obj to look at a per-mapper cascade flag of some kind.

[Mike Bayer]

this isn't really a MySQL issue, it's just that supports_sane_multi_rowcount can't be used for DELETEs. Not an issue on other DBs since they dont even support "multi row count".

diff -r 5448f6129cd0487c3d06324385cc2ef0701b5815 lib/sqlalchemy/orm/persistence.py
--- a/lib/sqlalchemy/orm/persistence.py Sat Mar 10 16:18:52 2012 -0800
+++ b/lib/sqlalchemy/orm/persistence.py Mon Mar 12 12:10:42 2012 -0700
@@ -642,12 +642,10 @@

     for connection, del_objects in delete.iteritems():
         statement = base_mapper._memo(('delete', table), delete_stmt)
-        rows = -1

         connection = cached_connections[connection]

-        if need_version_id and \
-                not connection.dialect.supports_sane_multi_rowcount:
+        if need_version_id:
             # TODO: need test coverage for this #1761
             if connection.dialect.supports_sane_rowcount:
                 rows = 0
@@ -656,6 +654,12 @@
                 for params in del_objects:
                     c = connection.execute(statement, params)
                     rows += c.rowcount
+                if rows != len(del_objects):
+                    raise orm_exc.StaleDataError(
+                        "DELETE statement on table '%s' expected to "
+                        "delete %d row(s); %d were matched." % 
+                        (table.description, len(del_objects), c.rowcount)
+                    )
             else:
                 util.warn(
                     "Dialect %s does not support deleted rowcount "
@@ -664,16 +668,8 @@
                     stacklevel=12)
                 connection.execute(statement, del_objects)
         else:
-            c = connection.execute(statement, del_objects)
-            if connection.dialect.supports_sane_multi_rowcount:
-                rows = c.rowcount
+            connection.execute(statement, del_objects)

-        if rows != -1 and rows != len(del_objects):
-            raise orm_exc.StaleDataError(
-                "DELETE statement on table '%s' expected to "
-                "delete %d row(s); %d were matched." % 
-                (table.description, len(del_objects), c.rowcount)
-            )

 def _finalize_insert_update_commands(base_mapper, uowtransaction, 
                             states_to_insert, states_to_update):

[Mike Bayer]

6c03a8ddd366b62285e2671a25a429f7bff1d052

[Mike Bayer]

Removing milestone: 0.7.6 (automated comment)

[Mike Bayer]

• 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 2403 for background on the original change. re: #2403 fix #3007

→ <<cset d6618e411955>>

[matt chisholm]

We are using Flask-SQLAlchemy and have classes that inherit from model classes, and this change seems to be causing us a lot of problems. However, I'm having trouble debugging it or producing a minimal test case, because it is not clear how to turn it off. The changelog says that this exception is just a warning, which seems incorrect: here it is raising an actual exception.The changelog also says that it can be configured on the mapper, but it's totally unclear how to do that (I have tried MyModel.my_relation.property.mapper.confirm_deleted_rows = False, and that seems to have no effect).

Could you confirm that this is actually just a warning, and maybe say how I could turn it off to verify that it's actually causing our problems?

[Mike Bayer]

code like this:

class MyClass(Base):
    # ...
    __mapper_args__ = {'confirm_deleted_rows': False}

also, it's a warning, unless you are using the version_id_col feature with that mapper, for which this has always been an exception. If it's raising, you want to look at the warnings filter: https://docs.python.org/2/library/warnings.html , otherwise show me a stack trace.

If you want a test case, look no further than the code right at the top of this issue. Just run it, at the bottom you get the warning:

/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/persistence.py:116: SAWarning: DELETE statement on table 'A' expected to delete 2 row(s); 1 were matched.  Please set confirm_deleted_rows=False within the mapper configuration to prevent this warning.
  cached_connections, mapper, table, delete)

[matt chisholm]

Actually, I was slightly wrong. We are hitting the StaleDataError in dependency.py, not this one. It is happening when emptying out a many-to-many relation (with a mapper table) on an object that is an instance of a subclass of a SQLAlchemy model class. It appears that SQLAlchemy is trying to delete the row in the mapper table twice, and getting a mismatch in rows the second time, because it's already been deleted. However, in my test case SA just tries to delete the row once. So it's probably related to our use of Flask-SQLAlchemy, so I have to add that to my test case. This is a new bug in 0.9.4, so I was looking around in recent SA changes to see if I could find what had changed to cause this.

I'm happy to make a new ticket with the traceback, but I figured you would want a functioning test case, so I'm trying to create that.

[Mike Bayer]

its not in 0.9.3? if so then the changes here are likely related still

[Mike Bayer]

although when you say "many-to-many relation (with a mapper table)", i really wish you'd show some code and/or stacktrace here but that sounds like you are using a mapped class' table as the argument in "secondary", which yes, if you change the state in two different places, SQLA doesn't know you're doing that and will delete, see the green note at the end of http://docs.sqlalchemy.org/en/latest/orm/relationships.html#association-object.

[matt chisholm]

Yep, that's what I mean by many-to-many. And yes, the problem is not manifesting in 0.9.3. I'll open a bug with a traceback and a minimal test case in the next few days. Sorry for the noise. :)

[matt chisholm]

traceback and minimal test case in #3015