Opened 4 years ago

Closed 2 years ago

#32095 closed Cleanup/optimization (fixed)

update_or_create should utilize update_fields on update

Reported by: Florian Apolloner Owned by: Sarah Boyce
Component: Database layer (models, ORM) Version: 3.1
Severity: Normal Keywords:
Cc: David Wobrock 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

update_or_create should update only the fields in default on update, not all fields. While it is concurrency-safe to update the whole model since update_or_create fetches the object via select_for_update it is still unnecessary to re transmit all fields back to the database (probably results in more wal/binary logs written etc etc…).

In the end update_or_create (and most likely get_or_create) might be written in more efficient ways depending on the database backend -- but update_fields seems to be a rather low-hanging fruit.

Change History (13)

comment:1 by Simon Charette, 4 years ago

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

It could have a noticeable impact on PostgreSQL to not update these columns in some scenario as are they are likely to be indexed if matched against (Write Amplification problem)

comment:2 by Liel Fridman, 4 years ago

Owner: changed from nobody to Liel Fridman
Status: newassigned

comment:3 by Mariusz Felisiak, 4 years ago

Has patch: set
Owner: changed from Liel Fridman to Florian Apolloner

comment:4 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:5 by Sarah Boyce, 2 years ago

Owner: changed from Florian Apolloner to Sarah Boyce
Patch needs improvement: unset

comment:6 by Mariusz Felisiak, 2 years ago

Patch needs improvement: set

comment:7 by Sarah Boyce, 2 years ago

Patch needs improvement: unset

Tentatively removing the patch_needs_improvement flag, there is a comment about clarifying existing documentation but not sure if that needs to be included in this PR :)

comment:8 by David Wobrock, 2 years ago

Cc: David Wobrock added

comment:9 by Sarah Boyce, 2 years ago

Patch needs improvement: set

comment:10 by Sarah Boyce, 2 years ago

Patch needs improvement: unset

comment:11 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 1d77b93:

Refs #32095 -- Added model's Options._non_pk_concrete_field_names property.

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 6cc0f22a:

Fixed #32095 -- Made QuerySet.update_or_create() save only fields passed in defaults or with custom pre_save().

Thanks Florian Apolloner for the initial patch.

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