Opened 6 months ago

Closed 5 months ago

Last modified 5 months ago

#36260 closed Bug (fixed)

bulk_create() does not fetch primary keys if they are generated by database

Reported by: Dmitry Shachnev Owned by: Dmitry Shachnev
Component: Database layer (models, ORM) Version: 5.2
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In our project, we use a function that generates UUID on database side (it’s uuid7 function from PostgreSQL 18).

The bulk_create function uses _is_pk_set() method to check if the object already has a primary key assigned. However, _is_pk_set() wrongly returns True in this case: primary key is not None, however it is a DatabaseDefault instance and does not actually return a value.

Because of this, the objects returned by bulk_create do not actually have an ID.

I prepared a patch to fix this, and wrote a test case for it. Without my patch, the test fails with the following error:

======================================================================
FAIL: test_db_default_primary_key (bulk_create.tests.BulkCreateTests.test_db_default_primary_key)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/dmitry/upstream/django/tests/bulk_create/tests.py", line 878, in test_db_default_primary_key
    self.assertIsInstance(obj1.id, UUID)
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
AssertionError: <django.db.models.expressions.DatabaseDefault object at 0x7f2563f6b570> is not an instance of <class 'uuid.UUID'>

----------------------------------------------------------------------

Change History (7)

comment:1 by Simon Charette, 6 months ago

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Not a release blocker as db_default was introduced in 5.0 and I could reproduce the issue against it.

comment:2 by Simon Charette, 6 months ago

Patch needs improvement: set

comment:3 by Simon Charette, 6 months ago

Patch needs improvement: unset

Patch LGTM. Left a comment about a potential improvement to object partitioning by primary key that could be done in a follow up optimization commit while we're around.

Last edited 6 months ago by Simon Charette (previous) (diff)

comment:4 by Sarah Boyce, 5 months ago

Triage Stage: AcceptedReady for checkin

comment:5 by Sarah Boyce <42296566+sarahboyce@…>, 5 months ago

Resolution: fixed
Status: assignedclosed

In 77b4ecb:

Fixed #36260 -- Made bulk_create() work with DB-generated primary keys.

Co-authored-by: Simon Charette <charette.s@…>

comment:6 by Sarah Boyce <42296566+sarahboyce@…>, 5 months ago

In 7d9aab8:

Refs #36260 -- Moved _is_pk_set checks into _prepare_for_bulk_create().

To avoid looping over objs twice.

comment:7 by GitHub <noreply@…>, 5 months ago

In 02e7a16:

Refs #36088, Refs #36260 - Added supports_expression_defaults checks in bulk_create() tests.

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