[Django] #28000: Migration forces "DROP DEFAULT" SQL
Django
-----------------------------------------------+------------------------
Reporter: Matteo Pietro Russo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------------+------------------------
If i consider to modify a column (ChaField) to add a default value as
follow:
from django.db import models
class PersonModel(models.Model):
nickname = models.CharField(default="noname", max_length=100,
null=False)
makemigration insists on creating a new migration for this change.
However, the migration on MYSQL shows:
BEGIN;
ALTER TABLE "core_personmodel" ALTER "nickname" SET DEFAULT 'noname';
ALTER TABLE "core_personmodel" ALTER "nickname" DROP DEFAULT;
COMMIT;
I think there is a mistake at line 735 in
django/db/backends/base/schema.py
{{{
# Drop the default if we need to
# (Django usually does not use in-database defaults)
if needs_database_default:
sql = self.sql_alter_column % {
"table": self.quote_name(model._meta.db_table),
"changes": self.sql_alter_column_no_default % {
"column": self.quote_name(new_field.column),
}
}
self.execute(sql)
}}}
If I "needs_database_default", why we need to drop default
(sql_alter_column_no_default)? I read we use it to prevent extraneous DROP
DEFAULT statements (an example is LONGTEXT in MySQL) because if database
is able to use default we need to avoid to drop it.
--
Ticket URL: <https://code.djangoproject.com/ticket/28000>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Matteo Pietro Russo:
Old description:
New description:
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:1>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: 1.9
Severity: Normal | Resolution: invalid
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 Tim Graham):
* status: new => closed
* resolution: => invalid
Comment:
Django uses database defaults to set values on existing rows in a table.
It doesn't leave the default values in the database so dropping the
default is correct behavior. There might be an optimization not to
set/drop the default in this case -- I'm not sure it's needed since the
column isn't null. A separate ticket could be opened for this.
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:2>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
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 Matteo Pietro Russo):
* status: closed => new
* resolution: invalid =>
Comment:
About default value, I read it in Django documentation:
>> "The default value is used when new model instances are created and a
value isn’t provided for the field."
It's right if I use Django ORM, but I expect my field has rule I set in my
model.
You said that "Django uses database defaults to set values on existing
rows in a table", but if I migrate my table adding default value,
migration doesn't change anything: It doesn't set default value on
existing rows, it' doesn't change field (it is nullable yet and i haven't
default).
{{{
from django.db import models
class PersonModel(models.Model):
nickname = models.CharField(max_length=100, null=True)
}}}
If I change it:
{{{
nickname = models.CharField(default="noname", max_length=100,
null=False)
}}}
makemigrations builds a migration with this operation:
{{{
migrations.AlterField(
model_name='core_personmodel',
name='nickname',
field=models.CharField(default="noname", max_length=100,
null=False),
),
}}}
If I execute migration:
{{{
Running migrations:
Rendering model states... DONE
Applying core.0004_migration... OK
}}}
but It's doesn't do anything.
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:3>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: 1.9
Severity: Normal | Resolution: invalid
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 Tim Graham):
* status: new => closed
* resolution: => invalid
Comment:
This is expected behavior. I should have said, "Django uses database
defaults to set values on existing *null* rows in a table."
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:4>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: 1.9
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Matteo Pietro Russo):
Ok, now I understand. For this reason, It's not a bug.
So I can't set default value on database via Django migrations, can I?
Default values are used by Django ORM, I think it's an useful improvement
if we can apply it in db via migrations (as soon as possible). Do you
agree?
MySQL backend uses base alter_field method so I can override it as
proposal.
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:5>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: 1.9
Severity: Normal | Resolution: needsinfo
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 Matteo Pietro Russo):
* resolution: invalid => needsinfo
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:6>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: 1.9
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
No, it's not acceptable to overwrite existing data the `default` without
the user's consent. You should use a `RunPython` operation to do that if
needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:7>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: 1.9
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Matteo Pietro Russo):
I don't want to overwrite anything: i want to set default value in my db
column (also if my table is empty).
{{{
ALTER TABLE "core_personmodel" ALTER "nickname" SET DEFAULT 'noname';
}}}
This statement doesn't modify data but column's specification, If I run it
and then I run django migration, It drops my previous SQL: this is a
mistake because I lost my defaults.
Why cannot I do that?
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:8>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: 1.9
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
> This statement doesn't modify data but column's specification, If I run
it and then I run django migration, It drops my previous SQL: this is a
mistake because I lost my defaults.
> Why cannot I do that?
Django's ORM don't use defaults defined at the database level and always
specify the value when performing inserts instead.
There have been [https://github.com/django/deps/pull/36 discussions to add
support for database generated field values] but nothing have been
implemented so far.
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:9>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: 1.9
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Matteo Pietro Russo):
Replying to [comment:9 Simon Charette]:
> > This statement doesn't modify data but column's specification, If I
run it and then I run django migration, It drops my previous SQL: this is
a mistake because I lost my defaults.
> > Why cannot I do that?
>
> Django's ORM don't use defaults defined at the database level and always
specify the value when performing inserts instead.
>
> There have been [https://github.com/django/deps/pull/36 discussions to
add support for database generated field values] but nothing have been
implemented so far.
This behaviour is correct but, I expect django migration doesn't delete my
column specification (it DROPS my defaults value).
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:10>
Django
--------------------------------------+------------------------------------
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: master
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* has_patch: 0 => 1
* type: Bug => Cleanup/optimization
* version: 1.9 => master
* stage: Unreviewed => Accepted
Comment:
Tim, I'll just repurpose this ticket for the optmization case.
[https://github.com/django/django/pull/8278 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:11>
Django
--------------------------------------+------------------------------------
Reporter: Matteo Pietro Russo | Owner: nobody
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: closed => new
* resolution: invalid =>
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:12>
Django
Reporter: Matteo Pietro Russo | Owner: nobody
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* status: new => closed
* resolution: => duplicate
Comment:
This is duplicate of #27928.
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:13>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Marcin Nowak):
> Django's ORM don't use defaults defined at the database level
But it should.
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:14>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Migrations | Version: dev
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Alessio Fachechi):
Replying to [comment:14 Marcin Nowak]:
> > Django's ORM don't use defaults defined at the database level
>
> But it should.
Totally agree. What's the reason for dropping them? I can understand the
benefits of making Django handle its own default logic, but that doesn't
mean dropping the database default, which can be used at last stage. Also
the Django app is not always the only app accessing and writing to a
database...
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:15>
Django
-------------------------------------+-------------------------------------
Cleanup/optimization |
Component: Migrations | Version: dev
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Leaving database defaults would be backward incompatible and could result
in surprising behavior. `Field.default` can be a callable which returns a
non-static value that could be inappropriate as a long-running default.
#470 is a new feature ticket to allow using database defaults.
--
Ticket URL: <https://code.djangoproject.com/ticket/28000#comment:16>