Opened 2 years ago

Closed 2 years ago

Last modified 22 months ago

#34217 closed Bug (fixed)

Migration removing a CheckConstraint results in ProgrammingError using MySQL < 8.0.16.

Reported by: Max Fisco Owned by: Bhuvnesh
Component: Migrations Version: 4.1
Severity: Normal Keywords:
Cc: David Wobrock Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

We are working on migrating from MySQL to PostgreSQL and a prerequisite step is to fix a CheckConstraint on an existing model:

class MyModel(models.Model):
    ...
    effective_date = models.DateField(verbose_name="Effective Date")
    expiration_date = models.DateField(verbose_name="Expiration Date")
    
    class Meta:
        constraints = [
            models.CheckConstraint(
                check=models.Q(expiration_date__gt=models.F("effective_date")),
                name="check_effective_date_before_expiration_date",
            ),
            ...
        ]

To do so we did the following:

  1. Removed the check constraint from the model's definition
  2. Made a new migration to drop the constraint
  3. Added the "fixed" check constraint to the model's definition
  4. Made a new migration to create the new constraint

Since check constraints are ignored on MySQL 5.7.31, we expected these two new migrations have no effect. Instead, we are encountering the following error:

MySQLdb.ProgrammingError: (1064, "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to
 use near 'CHECK `check_effective_date_before_expiration_date`' at line 1")

The sql code generated by the migration removing the constraint (/. manage.py sqlmigrate myapp 0002) produces the following:

-- Remove constraint check_effective_date_before_expiration_date from model mymodel
--
ALTER TABLE `myapp_mymodel` DROP CHECK `check_effective_date_before_expiration_date`;

So it seems that the migration is trying to drop the constraint even though it doesn't exist in the database. As a workaround, we can fake the "constraint removal" migration (./manage.py migrate --fake myapp 0002 ) but could this be a bug or are is there something we are missing?

Thanks!

Change History (11)

comment:1 by Mariusz Felisiak, 2 years ago

Summary: Migration removing a CheckConstraint results in ProgrammingError using MySQL (5.7.31)Migration removing a CheckConstraint results in ProgrammingError using MySQL < 8.0.16.
Triage Stage: UnreviewedAccepted

Thanks for the report. Agreed, removing a check constraint should be no-op when not supported, the following diff fix it for me:

  • django/db/backends/base/schema.py

    diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py
    index fe31967ce2..8e6c21647e 100644
    a b class BaseDatabaseSchemaEditor:  
    16671667        )
    16681668
    16691669    def _delete_check_sql(self, model, name):
     1670        if not self.connection.features.supports_table_check_constraints:
     1671            return None
    16701672        return self._delete_constraint_sql(self.sql_delete_check, model, name)
    16711673
    16721674    def _delete_constraint_sql(self, template, model, name):

Would you like to prepare a patch? (a regression test in migrations.test_operations.OperationTests is required.)

comment:2 by Max Fisco, 2 years ago

Hi Mariusz,

Thanks for the quick response! As far as the patch goes, I'd love to contribute (first time reporting a bug) but I think it'd be better to let someone else pick it up.

I tried to by following along with https://docs.djangoproject.com/en/dev/intro/contributing/#writing-your-first-patch-for-django; however, I'm having some difficulty figuring out how to just get the tests to run using MySQL 5.7 in the first place 😕.

  File "/Users/maxwellfisco/django-fork/django/django/db/backends/base/base.py", line 214, in check_database_version_supported
    raise NotSupportedError(
django.db.utils.NotSupportedError: MySQL 8 or later is required (found 5.7.40).

in reply to:  2 comment:3 by Mariusz Felisiak, 2 years ago

Replying to Max Fisco:

Hi Mariusz,

Thanks for the quick response! As far as the patch goes, I'd love to contribute (first time reporting a bug) but I think it'd be better to let someone else pick it up.

I tried to by following along with https://docs.djangoproject.com/en/dev/intro/contributing/#writing-your-first-patch-for-django; however, I'm having some difficulty figuring out how to just get the tests to run using MySQL 5.7 in the first place 😕.

  File "/Users/maxwellfisco/django-fork/django/django/db/backends/base/base.py", line 214, in check_database_version_supported
    raise NotSupportedError(
django.db.utils.NotSupportedError: MySQL 8 or later is required (found 5.7.40).

Django 4.2 no longer supports MySQL < 8, so you need to use MySQL 8 to test against the current main branch.

comment:4 by Bhuvnesh, 2 years ago

Hi Max, if you are working on this issue please assign it to yourself, if not, i would like to submit a patch. 🙂

in reply to:  4 comment:5 by Max Fisco, 2 years ago

Replying to Bhuvnesh:

Hi Max, if you are working on this issue please assign it to yourself, if not, i would like to submit a patch. 🙂

Hi Bhuvnesh, I apologize for leaving it ambiguous, go ahead and submit the patch.

comment:6 by Bhuvnesh, 2 years ago

Owner: changed from nobody to Bhuvnesh
Status: newassigned

Okay ,Thanks !

comment:7 by David Wobrock, 2 years ago

Cc: David Wobrock added

comment:8 by Jacob Walls, 2 years ago

Has patch: set

comment:9 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 68ef274b:

Fixed #34217 -- Fixed migration crash when removing check constraints on MySQL < 8.0.16.

comment:11 by GitHub <noreply@…>, 22 months ago

In 16c966ff:

Refs #30060, Refs #34217 -- Made SchemaEditor not generate SQL for CheckConstraint if not supported.

The new logic mirrors the logic in SchemaEditor._delete_check_sql()
added in 68ef274bc505cd44f305c03cbf84cf08826200a8.

Thanks Tim Graham for the report.

Note: See TracTickets for help on using tickets.
Back to Top