Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#23617 closed New feature (fixed)

The new UUID field does not allow to automatically generate a UUID on save

Reported by: Omer Katz Owned by: Rigel Di Scala
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: 1.8-beta
Cc: Marc Tamlyn Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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?

Change History (15)

comment:1 Changed 5 years ago by Tim Graham

Is there a problem with using default as documented?

comment:2 Changed 5 years ago by Rigel Di Scala

Owner: changed from nobody to Rigel Di Scala
Status: newassigned

comment:3 Changed 5 years ago by Rigel Di Scala

I could not replicate the issue on current master.

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

Last edited 5 years ago by Rigel Di Scala (previous) (diff)

comment:4 Changed 5 years ago by Rigel Di Scala

Resolution: worksforme
Status: assignedclosed

comment:5 Changed 5 years ago by Anssi Kääriäinen

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.

comment:6 Changed 5 years ago by Collin Anderson

Would that would be similar to auto_now_add?

comment:7 Changed 5 years ago by Omer Katz

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.

comment:8 Changed 5 years ago by Aymeric Augustin

Resolution: worksforme
Status: closednew

comment:9 Changed 5 years ago by Tim Graham

Cc: Marc Tamlyn added

There is 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.

comment:10 Changed 5 years ago by Marc Tamlyn

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.

comment:11 Changed 5 years ago by Anssi Kääriäinen

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()

comment:12 Changed 5 years ago 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

comment:13 Changed 5 years ago by Tim Graham

Has patch: set
Keywords: 1.8-beta added
Triage Stage: UnreviewedAccepted

comment:14 Changed 5 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 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.

comment:15 Changed 5 years ago by Tim Graham <timograham@…>

In 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

Note: See TracTickets for help on using tickets.
Back to Top