[Django] #36702: bulk_create not returning pk if it was set as an expression

[Django]

#36702: bulk_create not returning pk if it was set as an expression
-------------------------------------+-------------------------------------
Reporter: Johannes Westphal | Type:
| Uncategorized
Status: new | Component: Database
| layer (models, ORM)
Version: 5.2 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Currently, when the pk is set on an object passed to `bulk_create` the pk
is explicitly not updated in the object:

https://github.com/django/django/blob/05ba1a9228128614fb3c475f1c4bdf0160f44dba/django/db/models/query.py#L843

This becomes a problem if the pk of the object has been set to an
expression, because then we have no access to the actual generated pk even
though the db returned it.

Would it break any functionality to just remove the line above and also
update the pk even if it had been set?
--
Ticket URL: <https://code.djangoproject.com/ticket/36702>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36702: bulk_create not returning pk if it was set as an expression
-------------------------------------+-------------------------------------

Reporter: Johannes Westphal | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* stage: Unreviewed => Accepted
* type: Uncategorized => Bug

Comment:

Hello Jonathan, thank you for your report!

I went back to the [https://github.com/django/django/pull/9983 change that
introduced that line] and it goes all the way back to when we added
`RETURNING` support to `bulk_create` on Postgres (see #29444).

I couldn't any find mention of ''why'' we were doing it but I assume it
was initially as form of optimization or a way to make sure the primary
key preserves its exact identity

e.g.

- `UUID('70cf7ffa-000c-4dfd-b08b-a921f09e5ff2') == UUID('70cf7ffa-000c-
4dfd-b08b-a921f09e5ff2')`
- but `UUID('70cf7ffa-000c-4dfd-b08b-a921f09e5ff2') is not UUID('70cf7ffa-
000c-4dfd-b08b-a921f09e5ff2')`

Given the ORM makes favors equality of objects and hashability of objects
over identity I think it would be safe to remove this. The `bulk_create`
test suite also doesn't complain on Postgres and SQLite so I think this is
a safe change to make, would you like to submit a PR with a test for it?

FWIW I stumbled upon this particular case when working on #36490 to avoid
unnecessary transaction in `bulk_create`.
[https://github.com/django/django/pull/19614#issuecomment-3028756465 I
noticed] that there doesn't seem to be a reason to perform two with and
without pk partionned `INSERT` anymore.
[https://github.com/django/django/compare/main...charettes:django:ticket-36490
-simplify-bulk_create#diff-
d58ef61559dc7af5fdf7b56fee13571a4d2948e784cd608f6afeacf3ac2fb195R805 I
demonstrated that both branches can be merged together].
--
Ticket URL: <https://code.djangoproject.com/ticket/36702#comment:1>

[Django]

#36702: bulk_create not returning pk if it was set as an expression
-------------------------------------+-------------------------------------
Reporter: Johannes Westphal | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Johannes Westphal):

I pushed a
[https://github.com/django/django/compare/main...us77ipis:django:ticket_36702
test] for the bug, with your

[https://github.com/django/django/compare/main...charettes:django:ticket-36490
-simplify-bulk_create#diff-
d58ef61559dc7af5fdf7b56fee13571a4d2948e784cd608f6afeacf3ac2fb195R805

changes] the issue should already be solved.
--
Ticket URL: <https://code.djangoproject.com/ticket/36702#comment:2>

[Django]

#36702: bulk_create not returning pk if it was set as an expression
-------------------------------------+-------------------------------------
Reporter: Johannes Westphal | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* cc: Simon Charette (added)

Comment:

Thanks for the test!

I think we should merge your proposed `if field != opts.pk:` removal with
your test first though as my branch is more invasive and this ticket is
already focused on it.

Would you mind pushing the removal to your branch and open a PR with it?
Happy to review it one you do and we can trade reviews when I open the
follow up ticket and PR to optimize the insertion.
--
Ticket URL: <https://code.djangoproject.com/ticket/36702#comment:3>

[Django]

#36702: bulk_create not returning pk if it was set as an expression
-------------------------------------+-------------------------------------

Reporter: Johannes Westphal | Owner: Johannes
| Westphal
Type: Bug | Status: assigned

Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Johannes Westphal):

* owner: (none) => Johannes Westphal
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36702#comment:4>

[Django]

#36702: bulk_create not returning pk if it was set as an expression
-------------------------------------+-------------------------------------
Reporter: Johannes Westphal | Owner: Johannes
| Westphal
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Johannes Westphal):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/20041 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/36702#comment:5>

[Django]

#36702: bulk_create not returning pk if it was set as an expression
-------------------------------------+-------------------------------------
Reporter: Johannes Westphal | Owner: Johannes
| Westphal
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36702#comment:6>

[Django]

#36702: bulk_create not returning pk if it was set as an expression
-------------------------------------+-------------------------------------
Reporter: Johannes Westphal | Owner: Johannes
| Westphal

Type: Bug | Status: closed

Component: Database layer | Version: 5.2
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by GitHub <noreply@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"6d4d99b3cef4a6d931de02f89a493fb345dc438e" 6d4d99b3]:
{{{#!CommitTicketReference repository=""
revision="6d4d99b3cef4a6d931de02f89a493fb345dc438e"
Fixed #36702 -- Made bulk_create() return pk values set by an expression.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36702#comment:7>