[Django] #28715: Migration DateTimeField(auto_now_add=True -> default=django.utils.timezone.new)
Django
default=django.utils.timezone.new)
------------------------------------------+------------------------
Reporter: Дилян Палаузов | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.11
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 |
------------------------------------------+------------------------
A switch from DateTimeField(auto_now_add=True) to
DateTimeField(default=django.utils.timezone.new) creates the statements
ALTER TABLE SET DEFAULT '2017-10-16T09:35:52.710695'::timestamp;
ALTER TABLE DROP DEFAULT
which have no effects, apart from locking the whole table.
A proposal to recognize, when the effective default-callable doesn't
change:
{{{
diff --git a/django/db/backends/base/schema.py
b/django/db/backends/base/schema.py
--- a/django/db/backends/base/schema.py
+++ b/django/db/backends/base/schema.py
@@ -199,28 +199,32 @@ class BaseDatabaseSchemaEditor(object):
'requires_literal_defaults must provide a prepare_default()
method'
)
- def effective_default(self, field):
+ def effective_default_before_callable(self, field):
"""
- Returns a field's effective database default value
+ Returns a field's effective database default callable or value
"""
if field.has_default():
- default = field.get_default()
+ return field._get_default
elif not field.null and field.blank and
field.empty_strings_allowed:
if field.get_internal_type() == "BinaryField":
- default = six.binary_type()
+ return six.binary_type()
else:
- default = six.text_type()
+ return six.text_type()
elif getattr(field, 'auto_now', False) or getattr(field,
'auto_now_add', False):
default = datetime.now()
internal_type = field.get_internal_type()
if internal_type == 'DateField':
- default = default.date
+ return default.date
elif internal_type == 'TimeField':
- default = default.time
+ return default.time
elif internal_type == 'DateTimeField':
- default = timezone.now
- else:
- default = None
+ return timezone.now
+
+ def effective_default(self, field):
+ """
+ Returns a field's effective database default value
+ """
+ default = self.effective_default_before_callable(field)
# If it's a callable, call it
if callable(default):
default = default()
@@ -615,6 +619,7 @@ class BaseDatabaseSchemaEditor(object):
old_default != new_default and
new_default is not None and
not self.skip_default(new_field)
+ and self.effective_default_before_callable(old_field) !=
self.effective_default_before_callable(new
)
if needs_database_default:
if self.connection.features.requires_literal_defaults:
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28715>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Reporter: Дилян Палаузов | Owner: nobody
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
* type: Uncategorized => Cleanup/optimization
--
Ticket URL: <https://code.djangoproject.com/ticket/28715#comment:1>
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Дилян Палаузов:
Old description:
New description:
A switch from DateTimeField(auto_now_add=True) to
DateTimeField(default=django.utils.timezone.new) creates the statements
ALTER TABLE SET DEFAULT '2017-10-16T09:35:52.710695'::timestamp;
ALTER TABLE DROP DEFAULT
which have no effects, apart from locking the whole table twice.
A proposal to recognize, when the effective default-callable doesn't
change and skip changing the DEFAULT twice in this case, as well as not
generating a migration when this is the only change on a field:
{{{
diff --git a/django/db/backends/base/schema.py
b/django/db/backends/base/schema.py
--- a/django/db/backends/base/schema.py
+++ b/django/db/backends/base/schema.py
@@ -199,28 +199,33 @@ class BaseDatabaseSchemaEditor(object):
'requires_literal_defaults must provide a prepare_default()
method'
)
- def effective_default(self, field):
+ @staticmethod
+ def effective_default_before_callable(field):
BaseDatabaseSchemaEditor.effective_default_before_callable(field)
# If it's a callable, call it
if callable(default):
default = default()
@@ -615,6 +620,7 @@ class BaseDatabaseSchemaEditor(object):
old_default != new_default and
new_default is not None and
not self.skip_default(new_field)
+ and
BaseDatabaseSchemaEditor.effective_default_before_callable(old_field) !=
BaseDatabaseSchemaEdit
)
if needs_database_default:
if self.connection.features.requires_literal_defaults:
diff --git a/django/db/models/fields/__init__.py
b/django/db/models/fields/__init__.py
--- a/django/db/models/fields/__init__.py
+++ b/django/db/models/fields/__init__.py
@@ -1232,7 +1232,7 @@ class DateField(DateTimeCheckMixin, Field):
if self.auto_now:
kwargs['auto_now'] = True
if self.auto_now_add:
- kwargs['auto_now_add'] = True
+ kwargs['default'] = timezone.now
if self.auto_now or self.auto_now_add:
del kwargs['editable']
del kwargs['blank']
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28715#comment:2>
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Дилян Палаузов:
Old description:
> A switch from DateTimeField(auto_now_add=True) to
> DateTimeField(default=django.utils.timezone.new) creates the statements
> ALTER TABLE SET DEFAULT '2017-10-16T09:35:52.710695'::timestamp;
> ALTER TABLE DROP DEFAULT
> which have no effects, apart from locking the whole table twice.
>
> A proposal to recognize, when the effective default-callable doesn't
> change and skip changing the DEFAULT twice in this case, as well as not
> generating a migration when this is the only change on a field:
> {{{
> diff --git a/django/db/backends/base/schema.py
> b/django/db/backends/base/schema.py
> --- a/django/db/backends/base/schema.py
> +++ b/django/db/backends/base/schema.py
> @@ -199,28 +199,33 @@ class BaseDatabaseSchemaEditor(object):
> 'requires_literal_defaults must provide a prepare_default()
> method'
> )
>
> - def effective_default(self, field):
> + @staticmethod
> + def effective_default_before_callable(field):
> BaseDatabaseSchemaEditor.effective_default_before_callable(field)
> # If it's a callable, call it
> if callable(default):
> default = default()
> @@ -615,6 +620,7 @@ class BaseDatabaseSchemaEditor(object):
> old_default != new_default and
> new_default is not None and
> not self.skip_default(new_field)
> + and
> BaseDatabaseSchemaEditor.effective_default_before_callable(old_field) !=
> BaseDatabaseSchemaEdit
> )
> if needs_database_default:
> if self.connection.features.requires_literal_defaults:
> diff --git a/django/db/models/fields/__init__.py
> b/django/db/models/fields/__init__.py
> --- a/django/db/models/fields/__init__.py
> +++ b/django/db/models/fields/__init__.py
> @@ -1232,7 +1232,7 @@ class DateField(DateTimeCheckMixin, Field):
> if self.auto_now:
> kwargs['auto_now'] = True
> if self.auto_now_add:
> - kwargs['auto_now_add'] = True
> + kwargs['default'] = timezone.now
> if self.auto_now or self.auto_now_add:
> del kwargs['editable']
> del kwargs['blank']
> }}}
New description:
A switch from DateTimeField(auto_now_add=True) to
DateTimeField(default=django.utils.timezone.new) creates the statements
ALTER TABLE SET DEFAULT '2017-10-16T09:35:52.710695'::timestamp;
ALTER TABLE DROP DEFAULT
which have no effects, apart from locking the whole table twice.
A proposal to recognize, when the effective default-callable doesn't
change and skip changing the DEFAULT twice in this case, as well as not
generating a migration when this is the only change on a field:
{{{
diff --git a/django/db/backends/base/schema.py
b/django/db/backends/base/schema.py
--- a/django/db/backends/base/schema.py
+++ b/django/db/backends/base/schema.py
@@ -199,28 +199,33 @@ class BaseDatabaseSchemaEditor(object):
'requires_literal_defaults must provide a prepare_default()
method'
)
- def effective_default(self, field):
+ @staticmethod
+ def effective_default_before_callable(field):
BaseDatabaseSchemaEditor.effective_default_before_callable(field)
# If it's a callable, call it
if callable(default):
default = default()
@@ -615,6 +620,7 @@ class BaseDatabaseSchemaEditor(object):
old_default != new_default and
new_default is not None and
not self.skip_default(new_field)
+ and
BaseDatabaseSchemaEditor.effective_default_before_callable(old_field) !=
BaseDatabaseSchemaEdit
)
if needs_database_default:
if self.connection.features.requires_literal_defaults:
diff --git a/django/db/models/fields/__init__.py
b/django/db/models/fields/__init__.py
index 8d40c77..2f5d5c2 100644
--- a/django/db/models/fields/__init__.py
+++ b/django/db/models/fields/__init__.py
@@ -1232,7 +1232,7 @@ class DateField(DateTimeCheckMixin, Field):
if self.auto_now:
kwargs['auto_now'] = True
if self.auto_now_add:
- kwargs['auto_now_add'] = True
+ kwargs['default'] = datetime.date.today
if self.auto_now or self.auto_now_add:
del kwargs['editable']
del kwargs['blank']
@@ -1372,6 +1372,12 @@ class DateTimeField(DateField):
return []
+ def deconstruct(self):
+ name, path, args, kwargs = super(DateTimeField,
self).deconstruct()
+ if self.auto_now_add:
+ kwargs['default'] = timezone.now
+ return name, path, args, kwargs
+
def get_internal_type(self):
return "DateTimeField"
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28715#comment:3>
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Дилян Палаузов):
My plan is to work one more month with Django, so if there are concerns on
this patch, please raise them by then.
--
Ticket URL: <https://code.djangoproject.com/ticket/28715#comment:4>
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
The patch must apply cleanly to Django's master branch and it must have a
test. Ideally, you could provide the patch as a pull request and then
check "Has patch" on the ticket so that it appears in the review queue.
--
Ticket URL: <https://code.djangoproject.com/ticket/28715#comment:5>
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Дилян Палаузов):
* Attachment "auto_now_add_skip_sql_calls.patch" added.
Probably somebody with more experience will tweak the
test.migrations.test_autodetector.test_add_date_fields_with_auto_now_add_(not_asking_for_null_addition,
add_asking_for_default) tests, so that the line "kwargs['auto_now_add'] =
True" in django/db/models/fields/__init__.py can be removed. But
otherwise this shall be ready to go.
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Дилян Палаузов):
* has_patch: 0 => 1
* version: 1.11 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/28715#comment:6>
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Дилян Палаузов):
* Attachment "auto_now_add_skip_sql_calls.patch" added.
Probably somebody with more experience will tweak the
test.migrations.test_autodetector.test_add_date_fields_with_auto_now_add_(not_asking_for_null_addition,
add_asking_for_default) tests, so that the line "kwargs['auto_now_add'] =
True" in django/db/models/fields/__init__.py can be removed. But
otherwise this shall be ready to go.
--
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Дилян Палаузов):
* Attachment "auto_now_add_skip_sql_calls.patch" added.
Probably somebody with more experience will tweak the
test.migrations.test_autodetector.test_add_date_fields_with_auto_now_add_(not_asking_for_null_addition,
add_asking_for_default) tests, so that the line "kwargs['auto_now_add'] =
True" in django/db/models/fields/__init__.py can be removed. But
otherwise this shall be ready to go.
--
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Дилян Палаузов):
* Attachment "auto_now_add_skip_sql_calls.patch" added.
Probably somebody with more experience will tweak the
test.migrations.test_autodetector.test_add_date_fields_with_auto_now_add_(not_asking_for_null_addition,
add_asking_for_default) tests, so that the line "kwargs['auto_now_add'] =
True" in django/db/models/fields/__init__.py can be removed. But
otherwise this shall be ready to go.
--
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
Comment:
Sorry, I should have looked at the initial patch before I suggested
bringing it up to date. I don't think the approach is correct. I think
that a migration should be generated if `auto_now_add=True` changes to
`default=timezone.now` as they aren't entirely equivalent. The fix will
have to be at the `SchemaEditor` level. Given that `auto_now_add` and
`auto_now` might be deprecated (#22995), it might be a better use of time
to focus on that issue (although the patch forward isn't entirely clear).
--
Ticket URL: <https://code.djangoproject.com/ticket/28715#comment:7>
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Дилян Палаузов):
* Attachment "auto_now_add_skip_sql_calls.2.patch" added.
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Дилян Палаузов):
The purpose of the change is to tweak the migrations auto_now_add ->
default for DateField and DateTimeField not to call
ALTER TABLE SET DEFAULT '2017-10-16T09:35:52.710695'::timestamp;
ALTER TABLE DROP DEFAULT
towards the database. The attached auto_now_add_skip_sql_calls.2.patch
touches BaseDatabaseSchemaEditor, and does generate a migration, which
however skips modifying the DEFAULT.
auto_now_add being possibly deprecated means that this type of migration
will be created more often in the future and this patch accelerates the
execution of the migration (as it does not lock the table).
--
Ticket URL: <https://code.djangoproject.com/ticket/28715#comment:8>
Django
default=timezone.now from generating SQL
--------------------------------------+------------------------------------
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Changes (by Ülgen Sarıkavak):
* cc: Ülgen Sarıkavak (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/28715#comment:9>