[Django] #24182: Bug with functional default and migrations
Django
----------------------------+--------------------
Reporter: arveitch | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
I've found a bug on all of 1.7; I've tested up to 1.7.3. The code works
fine on 1.6. I think this is a reasonably common design pattern.
Here's the models.py:
{{{
import random
from django.db import models
def generateCode(prefix=''):
while True:
code = ''.join(
# Omit I, O, 1 and 0 as they can cause confusion
random.choice('ABCDEFGHJKLMNPQRSTUVWXYZ23456789')
for i in range(7)
)
code = prefix + code
if not PromotionalCode.objects.filter(code=code).exists():
break
return code
class PromotionalCode(models.Model):
code = models.CharField(
default=generateCode, db_index=True, max_length=12, unique=True
)
value_amount = models.DecimalField(
default=0, max_digits=7, decimal_places=2
)
}}}
./manage.py migrate gives this traceback:
{{{
Applying example.0001_initial...Traceback (most recent call last):
File "./manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/core/management/__init__.py", line 385, in
execute_from_command_line
utility.execute()
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/core/management/__init__.py", line 377, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/core/management/base.py", line 288, in run_from_argv
self.execute(*args, **options.__dict__)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/core/management/base.py", line 338, in execute
output = self.handle(*args, **options)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/core/management/commands/migrate.py", line 161, in
handle
executor.migrate(targets, plan, fake=options.get("fake", False))
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/migrations/executor.py", line 68, in migrate
self.apply_migration(migration, fake=fake)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/migrations/executor.py", line 102, in
apply_migration
migration.apply(project_state, schema_editor)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/migrations/migration.py", line 108, in apply
operation.database_forwards(self.app_label, schema_editor,
project_state, new_state)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/migrations/operations/models.py", line 36, in
database_forwards
schema_editor.create_model(model)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/backends/schema.py", line 213, in create_model
definition, extra_params = self.column_sql(model, field)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/backends/schema.py", line 125, in column_sql
default_value = self.effective_default(field)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/backends/schema.py", line 175, in
effective_default
default = field.get_default()
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/models/fields/__init__.py", line 719, in
get_default
return self.default()
File "/Users/andrew/Sites/testing/example/models.py", line 12, in
generateCode
if not PromotionalCode.objects.filter(code=code).exists():
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/models/query.py", line 606, in exists
return self.query.has_results(using=self.db)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/models/sql/query.py", line 457, in has_results
return compiler.has_results()
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/models/sql/compiler.py", line 757, in has_results
return bool(self.execute_sql(SINGLE))
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/models/sql/compiler.py", line 786, in execute_sql
cursor.execute(sql, params)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/backends/utils.py", line 81, in execute
return super(CursorDebugWrapper, self).execute(sql, params)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/backends/utils.py", line 65, in execute
return self.cursor.execute(sql, params)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/utils.py", line 94, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/backends/utils.py", line 65, in execute
return self.cursor.execute(sql, params)
File
"/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
/site-packages/django/db/backends/sqlite3/base.py", line 485, in execute
return Database.Cursor.execute(self, query, params)
django.db.utils.OperationalError: no such table: example_promotionalcode
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24182>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: arveitch | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by arveitch):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
New description:
I've found a bug on all of 1.7; I've tested up to 1.7.3. The code works
fine on 1.6 with syncdb. I think this is a reasonably common design
pattern.
If I comment out the PromotionalCode.objects.filter(code=code).exists()
test then it works fine on 1.7
--
--
Ticket URL: <https://code.djangoproject.com/ticket/24182#comment:1>
Django
Reporter: arveitch | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Migrations call the default function so that if there are any existing
rows, they can be populated with a value. I think you will need to adjust
the `generateCode()` function to return a value in the case that the
`PromotionalCode` table doesn't exist. I'll leave this open for a second
opinion in case I'm missing something.
--
Ticket URL: <https://code.djangoproject.com/ticket/24182#comment:2>
Django
Reporter: arveitch | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
I think that's right. This is a regression, but I think it was an
unavoidable one.
If this had been discovered before release, it certainly would have gone
into the backwards-incompatible section of the release notes for 1.7. Does
it make any sense to add it there now?
I also think we may need to consider whether there is any clean way, via
supported API, to detect the table-doesn't-exist yet case in a default
function like this one. Catching the exception is too broad, and asking
people to do their own raw SQL query to find out is quite ugly. Since this
is a backwards-incompatible regression I think if we can't fix it we would
ideally provide a clean, documented new alternative technique.
--
Ticket URL: <https://code.djangoproject.com/ticket/24182#comment:3>
Django
Reporter: arveitch | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by arveitch):
Thanks - I do then have a solution for my particular case which is to
query the PostgreSQL information schema using raw SQL to see if the table
has been created yet. My application only runs on Postgres so that's fine
for me.
Agree that a database agnostic utility to check if a table exists would be
much nicer as a general solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/24182#comment:4>
Django
Reporter: arveitch | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
You could use an undocumented (internal) API from the introspection to see
if the database table exists:
{{{#!python
In [1]: from django.db import connection
In [2]: connection.introspection.get_table_list(connection.cursor())
Out[2]: ['django_migrations', 'app_a_a1', 'app_a_a2']
}}}
Apart from that, your migration(s) will suffer from #23932, regardless of
whether you access the model or not.
1. Add the field with `unique=False, null=True, default=NOT_PROVIDED`
2. Propagate existing rows
3. Alter field to `unique=True, null=False, default=generateCode`
I don't think this, backwards incompatible, behavior explicitly needs to
be documented. Instead #23932 should be. That said, I'm inclined to close
this ticket as duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/24182#comment:5>
Django
-----------------------------+------------------------------------
Reporter: arveitch | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* version: 1.7 => master
* type: Bug => New feature
* stage: Unreviewed => Accepted
Comment:
It doesn't seem to me that the docs we added for #23932 really addressed
this.
--
Ticket URL: <https://code.djangoproject.com/ticket/24182#comment:6>
Django
-----------------------------+------------------------------------
Type: New feature | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by arveitch):
I've just tried this code in Django 1.8.2 and it now works as expected.
Looks like this bug has been solved sometime between 1.7.3 and 1.8.2
--
Ticket URL: <https://code.djangoproject.com/ticket/24182#comment:7>
Django
-----------------------------+------------------------------------
Type: New feature | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
Replying to [comment:7 arveitch]:
> I've just tried this code in Django 1.8.2 and it now works as expected.
>
> Looks like this bug has been solved sometime between 1.7.3 and 1.8.2
I confirm this is still an issue on master.
--
Ticket URL: <https://code.djangoproject.com/ticket/24182#comment:8>