Ticket #295 (closed defect: invalid)

Opened 5 years ago

Last modified 2 years ago

migrate command fails on model migrations with default values as callables

Reported by: flashingpumpkin@… Owned by: andrew
Priority: major Milestone:
Component: migrations Version: 0.6.2
Keywords: migration, default_value, callable Cc:

Description

Given the model MyModel?, I've added a new field with a default_value=<callable> field, in the below example a foreign key - but it really does not matter. It could be a models.CharField? too.

from django.contrib.sites.models import Site

class MyModel(models.Model):
    ...fields...
    site = models.ForeignKey(Site, default_value=Site.objects.get_current)

The 'python manage.py migrate <app name>' command fails.

Here's the traceback:

(challenge)alen@false-positive:[challenge]$ python manage.py startmigration app schedule --auto
 ~ Changed field 'app.subscriptionschedule.site'.
 ~ Changed field 'app.subscription.site'.
 Created 0016_schedule.py.
(challenge)alen@false-positive:[challenge]$ python manage.py migrate app
Running migrations for app:
 - Migrating forwards to 0016_schedule.
 > app: 0016_schedule
Traceback (most recent call last):
  File "/home/alen/projects/challenge/lib/python2.6/site-packages/south/migration.py", line 329, in run_migrations
    runfunc(orm)
  File "/home/alen/projects/challenge/src/app/app/migrations/0016_schedule.py", line 15, in forwards
    db.alter_column('app_subscriptionschedule', 'site_id', orm['app.subscriptionschedule:site'])
  File "/home/alen/projects/challenge/lib/python2.6/site-packages/south/db/generic.py", line 311, in alter_column
    self.execute("ALTER TABLE %s %s;" % (qn(table_name), sql), values)
  File "/home/alen/projects/challenge/lib/python2.6/site-packages/south/db/generic.py", line 86, in execute
    cursor.execute(sql, params)
  File "/home/alen/projects/challenge/lib/python2.6/site-packages/django/db/backends/util.py", line 19, in execute
    return self.cursor.execute(sql, params)
  File "/home/alen/projects/challenge/lib/python2.6/site-packages/django/db/backends/mysql/base.py", line 84, in execute
    return self.cursor.execute(query, args)
  File "/usr/local/lib/python2.6/dist-packages/MySQL_python-1.2.3c1-py2.6-linux-i686.egg/MySQLdb/cursors.py", line 173, in execute
    self.errorhandler(self, exc, value)
  File "/usr/local/lib/python2.6/dist-packages/MySQL_python-1.2.3c1-py2.6-linux-i686.egg/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
OperationalError: (1067, "Invalid default value for 'site_id'")
 ! Error found during real run of migration! Aborting.

 ! Since you have a database that does not support running
 ! schema-altering statements in transactions, we have had to
 ! leave it in an interim state between migrations.

 ! You *might* be able to recover with:
   = ALTER TABLE `app_subscriptionschedule` ; []
   = ALTER TABLE `app_subscriptionschedule` ALTER COLUMN `site_id` DROP DEFAULT; []
   = ALTER TABLE `app_subscriptionschedule` MODIFY `site_id` integer NULL;; []
   = ALTER TABLE `app_subscription` ; []
   = ALTER TABLE `app_subscription` ALTER COLUMN `site_id` DROP DEFAULT; []
   = ALTER TABLE `app_subscription` MODIFY `site_id` integer NULL;; []

 ! The South developers regret this has happened, and would
 ! like to gently persuade you to consider a slightly
 ! easier-to-deal-with DBMS.
(challenge)alen@false-positive:[challenge]$


I reckon the reason for this fail is the fact that I'm passing a callable in to get the default value from. I've observed this behaviour previously.

~alen

Change History

comment:1 Changed 5 years ago by andrew

  • Status changed from new to closed
  • Resolution set to invalid

Yes, please don't pass in a callable in for the default value. South 0.7 will pretty much solve the behaviour of defaults, but for now:

  • Try not to use callables for defaults. If you do, South will call them at migration-creation time and store the value they return (this is NOT IDEAL for foreign keys)
  • If you must add an FK column, first add it as null=True, then write a data migration to populate it with the correct default values, then write one which alters it to null=False.

comment:2 Changed 4 years ago by matt@…

It's also not that useful where I am wanting UUIDs, and want to use a callable (uuid.uuid4) as the default.

What I have been doing is removing the default key from the migration dictionary for that field. Perhaps this could be a solution where the default on a column is callable? Just don't add in the default.

comment:3 follow-up: ↓ 4 Changed 4 years ago by andrew

Yes, but if you pass a callable as the default, how am I supposed to pass that to the database, which is expecting a single SQL-encoded value for the default value?

There is something to be said for better handling of callable defaults - as you suggest, just leaving it nullable, and automating filling it out later - but that's more of a post-1.0 feature.

comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 4 years ago by steven.davidson@…

It seems as though as long as your default callable returns a string rather than something that Django would eventually coerce into a string, it will be fine (on 0.7.2, I think). So:

def uuid():
    return str(uuid.uuid4())

class MyModel(models.Model):    
    my_field = models.CharField(max_length=40, default=uuid)

works.

Great utility though, thanks for developing!

Steven.

comment:5 in reply to: ↑ 4 Changed 4 years ago by steven.davidson@…

Replying to steven.davidson@…:

Of course, avoiding a name clash with uuid itself would be the thing to do ;-) D'oh.

def str_uuid():
    return str(uuid.uuid4())

class MyModel(models.Model):    
    my_field = models.CharField(max_length=40, default=str_uuid)


comment:6 Changed 4 years ago by andrew

Yes, that will work, but South will give the same uuid value to all rows if you do that for a column you're adding, since it gets called once then frozen into the SQL statement, so it's not recommended.

comment:7 Changed 4 years ago by matt@…

Which is actually worse than the current behaviour.

Now, when I have a uuid column, it fails to migrate unless I remove the default arg from the frozen model dict. Once I do this, everything is fine.

comment:8 Changed 3 years ago by matt@…

I actually have a solution for this now.

On my UUIDField, in the add_intropection_rules, I have a kwargs:

"default": ["default", {"ignore_if": "default"}],

This prevents the creation of the default entry for this field.

comment:9 Changed 2 years ago by anonymous

Why does south have to freeze after calling once? I'm running into this same issue with a UserProfile? vanity field that defaults to a UUID. If it's completely allowable in Django, why not make south handle it? Seems like this should be reopened.

comment:10 Changed 2 years ago by andrew

As I described above, it's because the default is used for an ADD COLUMN call, and the value provided there must be a single, SQL-encodeable value. If you can find a sensible way of supplying arbitary Python functions to the databases over SQL, then perhaps this bug is worth revisiting, but I suspect that won't happen :)

Note: See TracTickets for help on using tickets.