Change in behaviour when saving a model instance with an explcit pk value if the pk field has a default

[Reupen Shah]

Consider the following model:

from uuid import uuid4

from django.db import models

class Sample(models.Model):
    id = models.UUIDField(primary_key=True, default=uuid4)
    name = models.CharField(blank=True, max_length=100)

In Django 2.2 and earlier, the following commands would result in an INSERT followed by an UPDATE:

s0 = Sample.objects.create()
s1 = Sample(pk=s0.pk, name='Test 1')
s1.save()

However, in Django 3.0, this results in two INSERTs (naturally the second one fails). The behaviour also changes if default=uuid4 is removed from the id field.

This seems related to https://code.djangoproject.com/ticket/29260.

The change in behaviour also has the side effect of changing the behaviour of the loaddata management command when the fixture contains explicit pk values and the objects already exist (e.g. when loading the fixture multiple times).

Perhaps the intention was to only change the behaviour if an explicit pk value was not set on the model instance being saved? (At least, that would be more backwards-compatible behaviour...)

[Simon Charette]

It looks like ​the logic in _save_table should not force an insert if an explicit pk value is provided.

The logic should likely take pk_set into account

if (
    not pk_set
    and self._state.adding
    and self._meta.pk.default
    and self._meta.pk.default is not NOT_PROVIDED
):
    force_insert = True

I'm surprised this was not caught by the suite if this breaks fixtures loading.

[Baptiste Mispelon]

I bisected the regression down to 85458e94e38c20e57939947ee515a1a53689659f if that helps.

[Simon Charette]

I'm afraid we'll have to revert 85458e94e38c20e57939947ee515a1a53689659f if we want this pattern to work because we assign field defaults on Model.__init__ without tracking whether or not they were generated from field defaults or not which makes both Sample() and Sample(pk=s0.pk) have pk_set=True in _save_table.

Note that this limitation was mentioned in https://code.djangoproject.com/ticket/29260#comment:3 so another option could be to document that force_update must be used in this particular case. I feel like this would be good compromise. Regarding the fixture loading we should branch of raw ​which is passed by the serialization framework to disable the optimiation.

Happy to provide a patch for whatever solution we choose. I think it's worth adjusting the feature given and documenting it as a backward incompatible change that be worked around by passing force_update but simply reverting the feature and reopening #29260 to target the next release is likely less trouble.

[Reupen Shah]

If it helps, I noticed this through the changed behaviour of loaddata in such cases (rather than the example code given). (I don't think we have any code that directly looks like the example.)

[Mariusz Felisiak]

Replying to Simon Charette:

Note that this limitation was mentioned in https://code.djangoproject.com/ticket/29260#comment:3 so another option could be to document that force_update must be used in this particular case. I feel like this would be good compromise. Regarding the fixture loading we should branch of raw ​which is passed by the serialization framework to disable the optimiation.

Happy to provide a patch for whatever solution we choose. I think it's worth adjusting the feature given it does reduce the number of queries significantly when using primary key defaults and documenting it as a backward incompatible change that can be worked around by passing force_update...

I really like this proposition and I think it's fine to adjust the current fix and backport it to the Django 3.0.

[Simon Charette]

​https://github.com/django/django/pull/12209

[Mariusz Felisiak <felisiak.mariusz@…>]

In 5779cc93:

Fixed #31071 -- Disabled insert optimization for primary keys with defaults when loading fixtures.

Model.save_base() is called directly when loading fixtures and assumes
existing rows will be updated. Branching of "raw" allows to maintain
the optimization introduced in #29260 while supporting this edge case.

Regression in 85458e94e38c20e57939947ee515a1a53689659f.

Thanks Reupen Shah for the report.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7db4ab84:

[3.0.x] Fixed #31071 -- Disabled insert optimization for primary keys with defaults when loading fixtures.

Model.save_base() is called directly when loading fixtures and assumes
existing rows will be updated. Branching of "raw" allows to maintain
the optimization introduced in #29260 while supporting this edge case.

Regression in 85458e94e38c20e57939947ee515a1a53689659f.

Thanks Reupen Shah for the report.

Backport of 5779cc938a34eb96815c7a40ded2c8f6c8087c58 from master