[Django] #23617: The new UUID field does not allow to automatically generate a UUID on save
Django
----------------------------------------------+--------------------
Reporter: thedrow | Owner: nobody
Type: New feature | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
When using a UUID as a primary key it can be very convenient to have a
UUID generated for you much like the AutoNumber field does on save.
Currently it does not.
Should we use `auto` and `auto_add` keyword arguments like the Date*Field
or should we have a AutoUUIDField that is used only for primary keys?
--
Ticket URL: <https://code.djangoproject.com/ticket/23617>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: thedrow | Owner: nobody
Type: New feature | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Is there a problem with using `default`
[https://docs.djangoproject.com/en/dev/ref/models/fields/#django.db.models.UUIDField
as documented]?
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:1>
Django
Reporter: thedrow | Owner: zedr
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by zedr):
* status: new => assigned
* owner: nobody => zedr
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:2>
Django
Reporter: thedrow | Owner: zedr
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by zedr):
I could not replicate the issue.
I followed the example provided in the Django Developer Docs:
{{{
class MyUUIDModel(models.Model):
id = models.UUIDField(primary_key=True, default=uuid.uuid4,
editable=False)
}}}
and it works as intended. Instances of the MyUUIDModel will have a valid
and unique UUID4 identifier created by default, if one is not provided.
I did not find an existing test for this feature. To investigate this, I
created a new test for the UUIDField default keyword, and submitted it as
https://github.com/django/django/pull/3351
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:3>
Django
Reporter: thedrow | Owner: zedr
Component: Database layer | Version: master
(models, ORM) | Resolution:
Keywords: | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by zedr):
* status: assigned => closed
* resolution: => worksforme
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:4>
Django
Reporter: thedrow | Owner: zedr
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | worksforme
Keywords: | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by akaariai):
There is a distinction between how AutoField and UUIDField work. For
pk=AutoField `MyModel(pk=None, another_field='some_value').save()` will
create a new pk value, but for pk=UUIDField `MyModel(pk=None,
another_field='some_value').save()` will try to save the None value to the
database, resulting in primary key constraint error. IMO it would be
better if UUIDField worked similarly to AutoField. This could be achieved
by generating a new uuid value before save if the pk value is None.
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:5>
Django
Reporter: thedrow | Owner: zedr
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | worksforme
Keywords: | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by collinanderson):
Would that would be similar to `auto_now_add`?
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:6>
Django
Reporter: thedrow | Owner: zedr
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | worksforme
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by thedrow):
Why was this closed?
We need to resolve this before 1.8 is released.
It's simply a matter of adding a default value or documenting that you
have to set the default.
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:7>
Django
Reporter: thedrow | Owner: zedr
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* status: closed => new
* resolution: worksforme =>
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:8>
Django
Reporter: thedrow | Owner: zedr
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: mjtamlyn (added)
Comment:
There is
[https://docs.djangoproject.com/en/dev/ref/models/fields/#django.db.models.UUIDField
documentation that suggests] to use `default=uuid.uuid4`, but I'll leave
the ticket open for Marc to respond to Anssi on whether or not we want to
change this.
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:9>
Django
Reporter: thedrow | Owner: zedr
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mjtamlyn):
The main difference between `AutoField` and a `UUIDField` configured to
act as the primary key is that an `AutoField` is allowed to ask the
database for its value. Because Django does not support database level
defaults in general, this is not possible for a `UUIDField`.
If we are going to fully support alternative fields as `AutoField`-like
fields (see #14286 for example) then we should do it "properly" and allow
database level defaults, using `RETURNING` etc. This is a much more
difficult change involving a lot of work in the migrations framework as
well as the model layer.
I'm not in favour of adding more hackery to `UUIDField` specifically to
deal with this situation - I don't feel it is specific enough to this
particular field.
I'm opening a documentation change to mention that setting `pk = None` is
not necessarily a reliable way of generating a copy when the primary key
is not database generated.
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:10>
Django
Reporter: thedrow | Owner: zedr
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
I recall presenting an alternate way to handle this. The original idea is
probably in the PR for the UUID field.
The idea was that we add a new method to fields, something like
get_value_on_save(). If the field implements this method, and you are
saving an instance with pk=None to the database, then the
get_value_on_save() method would be called. Thus, when UUIDField's value
is None, then a new UUID would be generated.
This would be a better solution than what we have currently for two
reasons:
- This would allow adding an UUIDField to models in migrations (the same
default value wouldn't be used for every instance)
- Model(uuid_pk=None).save() would save with an autogenerated UUID
value, same with instance.pk = None; insance.save()
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:11>
Django
Reporter: thedrow | Owner: zedr
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by synotna):
As far as I understand it would be fixed by the proposal, but I have not
seen it explicitly mentioned yet in any discussion: In 1.8a1 following the
docs method, obj.pk is set before the obj is saved in the database, which
means testing if the obj is already saved with obj.pk no longer works
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:12>
Django
Reporter: thedrow | Owner: zedr
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* keywords: => 1.8-beta
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
[https://github.com/django/django/pull/3999 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:13>
Django
Reporter: thedrow | Owner: zedr
Component: Database layer | Version: master
(models, ORM) |
Keywords: 1.8-beta | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"8adc59038cdc6ce4f9170e4de2d716d940e136b3"]:
{{{
#!CommitTicketReference repository=""
revision="8adc59038cdc6ce4f9170e4de2d716d940e136b3"
Fixed #23617 -- Added get_pk_value_on_save()
The method is mainly intended for use with UUIDField. For UUIDField we
want to call the field's default even when primary key value is
explicitly set to None to match the behavior of AutoField.
Thanks to Marc Tamlyn and Tim Graham for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:14>
Django
Reporter: thedrow | Owner: zedr
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: 1.8-beta | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"43b0131fb5724cb92716eedfe35a6b2b4d84e1e5"]:
{{{
#!CommitTicketReference repository=""
revision="43b0131fb5724cb92716eedfe35a6b2b4d84e1e5"
[1.8.x] Fixed #23617 -- Added get_pk_value_on_save()
The method is mainly intended for use with UUIDField. For UUIDField we
want to call the field's default even when primary key value is
explicitly set to None to match the behavior of AutoField.
Thanks to Marc Tamlyn and Tim Graham for review.
Backport of 8adc59038cdc6ce4f9170e4de2d716d940e136b3 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23617#comment:15>