Opened 5 hours ago
Last modified 75 minutes ago
#36702 new Bug
bulk_create not returning pk if it was set as an expression
| Reported by: | Johannes Westphal | Owned by: | |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 5.2 |
| Severity: | Normal | Keywords: | |
| Cc: | Johannes Westphal | Triage Stage: | Accepted |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Currently, when the pk is set on an object passed to bulk_create the pk is explicitly not updated in the object:
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?
Change History (1)
comment:1 by , 75 minutes ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Uncategorized → Bug |
Note:
See TracTickets
for help on using tickets.
Hello Jonathan, thank you for your report!
I went back to the change that introduced that line and it goes all the way back to when we added
RETURNINGsupport tobulk_createon 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')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_createtest 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. I noticed that there doesn't seem to be a reason to perform two with and without pk partionnedINSERTanymore. I demonstrated that both branches can be merged together.