Opened 3 years ago

Last modified 3 years ago

#33312 assigned Cleanup/optimization

Instances with deferred fields cannot be used for copying.

Reported by: Adam Sołtysik Owned by: Kuldeep Pisda
Component: Database layer (models, ORM) Version: 3.2
Severity: Normal Keywords:
Cc: Egor R Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As per 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.

Change History (4)

comment:1 by Egor R, 3 years ago

Cc: Egor R added

comment:2 by Mariusz Felisiak, 3 years ago

Summary: Errors when trying to clone an object from a deferred querysetInstances with deferred fields cannot be used for copying.
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

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

comment:3 by Adam Sołtysik, 3 years ago

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.

comment:4 by Kuldeep Pisda, 3 years ago

Owner: changed from nobody to Kuldeep Pisda
Status: newassigned
Note: See TracTickets for help on using tickets.
Back to Top