Savepoint support for MySQL backend

[Chris Lamb]

Currently, it is not possible for the following code to deadlock when using the MySQL backend:

from django.db import models, transaction

class Parent(models.Model):
    lookup = models.CharField(max_length=20, unique=True)
    num_children = models.IntegerField(default=0)

class Child(models.Model):
    parent = models.ForeignKey(Parent, related_name='children')
    name = models.CharField(max_length=20)

    def save(self, *args, **kwargs):
        created = not self.pk

        super(Child, self).save(*args, **kwargs)

        if created:
            Parent.objects.filter(pk=self.parent_id).update(
                num_children=models.F('num_children') + 1,
            )

@transaction.commit_on_success
def my_view(request):
    parent = Parent.objects.get_or_create(lookup='a')
    parent.children.create(name='foo')

This is because the INSERT in get_or_create gains an IX lock on that row even though it fails and raises IntegrityError. This lock causes the deadlock later on when UPDATE is called from Child.save.

Adding savepoint support would fix this as the rollback to savepoint made just before the INSERT would remove the IX lock.

Patch attached. Tested on 5.0.84-1.

[Russell Keith-Magee]

If MySQL supports it, we should too.

The patch looks good, too -- but are you sure that the feature check is correct? Looking at MySQL docs, it looks like savepoints became available at 5.0.3 -- but your feature check only inspects for 5.0. It's also a feature that is only available for InnoDB.

[Chris Lamb]

You are right about 5.0.3 being the minimum version required, not 5.0 - I didn't see that paragraph in the manual. I'll update the patch now.

We shouldn't worry about savepoints only being available for InnoDB as we don't even turn off COMMIT/ROLLBACK calls when doing queries against MyISAM tables. Besides, we can't detect "for InnoDB" it on the database connection level as it's a property of the table.

[Russell Keith-Magee]

Replying to lamby:

You are right about 5.0.3 being the minimum version required, not 5.0 - I didn't see that paragraph in the manual. I'll update the patch now.

We shouldn't worry about savepoints only being available for InnoDB as we don't even turn off COMMIT/ROLLBACK calls when doing queries against MyISAM tables. Besides, we can't detect "for InnoDB" it on the database connection level as it's a property of the table.

Granted -- I just want to make sure that it's truly a no-op. MySQL MyISAM ignores transactions, and other than not having transactional behavior, it doesn't affect Django's internals. However, some of Django's internals use savepoints (get_or_create, for instance), so we need to make sure adding this behavior doesn't have other consequences on databases that don't support it.

[Tomasz Zieliński]

Have you verified that MySQLdb supports savepoints? I'm pretty sure that once I checked that and it didn't.

[Chris Lamb]

I just want to make sure that it is truly a no-op.

From my tests it doesn't interfere with MyISAM although I have a limited number of MySQL versions I can test with. I did test it with innodb completely disabled with skip-innodb and that was fine; hopefully this is functionally equivalent to not compiling InnoDB in.

Have you verified that MySQLdb supports savepoints? I'm pretty sure that once I checked that and it didn't.

We just execute "SAVEPOINT foo" SQL, no special support required (unless I'm missing something?).

[Tomasz Zieliński]

Replying to lamby:

Have you verified that MySQLdb supports savepoints? I'm pretty sure that once I checked that and it didn't.

We just execute "SAVEPOINT foo" SQL, no special support required (unless I'm missing something?).

Ah ok, then I might have missed something, I don't remember the details now.

[Chris Lamb]

(Updated the patch a few days ago which corrected the feedback, forgot to unset "patch needs improvements")

[Julien Phalip]

See #13742 which asks for savepoints support with sqlite.

[Ramiro Morales]

Patch updated with tests and doc modifications

[Ramiro Morales]

New patch, updated to apply and with skipping of a test when MySQL+MyISAM is in use

[Ramiro Morales]

In [17341]:

Added support for savepoints to the MySQL DB backend.

MySQL provides the savepoint functionality starting with version 5.0.3
when using the MyISAM storage engine.

Thanks lamby for the report and patch.

Fixes #15507.

[Ramiro Morales]

In [17923]:

Added documentation notes about lack of database savepoints support when using MySQL+MyISAM.

Refs #15507, #18116 and r17341, r17921.

[Ramiro Morales]

In [17924]:

[1.4.X] Added documentation notes about lack of database savepoints support when using MySQL+MyISAM.

Refs #15507 and r17341.

Backport of r17923.