[Django] #33312: Errors when trying to clone an object from a deferred queryset
Django
-------------------------------------+-------------------------------------
Reporter: Adam | Owner: nobody
Sołtysik |
Type: Bug | Status: new
Component: Database | Version: 3.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
As per [https://docs.djangoproject.com/en/3.2/topics/db/queries/#copying-
model-instances documentation], it's possible to clone objects by setting
`pk = None`.
However, if the object to clone comes from a deferred queryset (after
calling `defer` or `only`), trying to save the copy raises some hard to
decipher exceptions.
{{{
class DeferCloneTest(TestCase):
@classmethod
def setUpTestData(cls):
SimpleItem.objects.create(name="test", value=42)
def _get_item(self):
item = SimpleItem.objects.defer('value').first() # same with
`only` instead of `defer`
item.pk = None
item._state.adding = True # doesn't seem to change anything
return item
def test_save(self):
self._get_item().save() # ValueError: Cannot force an update in
save() with no primary key.
def test_save_force_insert(self):
self._get_item().save(force_insert=True) #
SimpleItem.DoesNotExist
def test_bulk_create(self):
SimpleItem.objects.bulk_create([self._get_item()]) #
SimpleItem.DoesNotExist
}}}
Possibly related: #27419, #28019.
--
Ticket URL: <https://code.djangoproject.com/ticket/33312>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Egor R (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/33312#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Sołtysik | Owner: nobody
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Thanks for the report. Instances with deferred fields cannot be used for
copying model instances, because ''"When calling save() for instances with
deferred fields, only the loaded fields will be saved"'' (see
[https://docs.djangoproject.com/en/3.2/ref/models/querysets/#defer docs]).
It's niche and I don't think it's worth changing, or even if it's feasible
with backward compatibility, we can add a warning to the ''"Copying model
instances"'' section, e.g. `Instances with deferred fields cannot be used
....`.
--
Ticket URL: <https://code.djangoproject.com/ticket/33312#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Sołtysik | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Adam Sołtysik):
Ok, I understand that it might not make much sense to implement copying
instances with deferred fields. A warning in the docs would certainly be
helpful. Adding a more readable exception message (consistent for all the
cases) may also be worth considering, as sometimes it's not obvious that
our queryset has deferred fields, like when there's a custom manager
defined for a model. Especially the `DoesNotExist` exception in the last
two cases in very unclear.
--
Ticket URL: <https://code.djangoproject.com/ticket/33312#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => kdpisda
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33312#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Charette
Cleanup/optimization | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Kuldeep Pisda => Simon Charette
--
Ticket URL: <https://code.djangoproject.com/ticket/33312#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Sołtysik | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33312#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Sołtysik | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33312#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Sołtysik | Owner: Simon
Type: | Charette
(models, ORM) |
| checkin
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* resolution: => fixed
* status: assigned => closed
Comment:
In [changeset:"e03e5c751c56db5f4cb99e142c92d7d8db3a5463" e03e5c75]:
{{{#!CommitTicketReference repository=""
revision="e03e5c751c56db5f4cb99e142c92d7d8db3a5463"
Fixed #33312 -- Raised explicit exception when copying deferred model
instances.
Previously save() would crash with an attempted forced update message, and
both
save(force_insert=True) and bulk_create() would crash with DoesNotExist
errors
trying to retrieve rows with an empty primary key (id IS NULL).
Implementing deferred field model instance copying might be doable in
certain
cases (e.g. when all the deferred fields are db generated) but that's not
trivial to implement in a backward compatible way.
Thanks Adam Sołtysik for the report and test and Clifford for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33312#comment:8>