Opened 9 years ago

Closed 4 days ago

Last modified 16 hours ago

#27222 closed New feature (fixed)

Refresh fields that are expressions after Model.save()

Reported by: holvianssi Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 1.10
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 (last modified by holvianssi)

The use case is automatically fetching the value for expressions when saving to DB. For example:

> user = User.objects.create(username=Lower('Anssi'))
> user.username == 'anssi'
True

While the above feature can be simulated somewhat easily by calling refresh_from_db() after save, an in-built implementation has the ability to use RETURNING as an optimization. In addition, it seems that refreshing objects on save would be a nice default, though this might be a bit backwards incompatible in some cases.

Change History (18)

comment:1 by holvianssi, 9 years ago

Description: modified (diff)

comment:2 by Tim Graham, 9 years ago

Summary: Refresh expressions on saveRefresh fields that are expressions after Model.save()
Triage Stage: UnreviewedAccepted

Does this also fix #23386? In that case, we might close this as a duplicate.

comment:3 by Simon Charette, 9 years ago

Has patch: set

comment:4 by Simon Charette, 9 years ago

This might also be related to #21454 or at least to the implementation proposed in it's PR as we should really be using RETURNING on backends that support it instead of performing a second SELECT to fetch the database generated fields.

comment:5 by holvianssi, 9 years ago

I'm hesitant to go with RETURNING implementation for the first patch. The select approach is really simple, and it will be needed in any case for some backends. The RETURNING approach on the other hand will be complex, and after all it's just an optimisation.

This should almost solve #21454 with a Default expression. This would need to be assigned manually to fields pre-save. Then full solution to #21454 would be adding a bit of API to do the pre-save assignments automatically.

And yes, I believe this solves #23386.

comment:6 by Tim Graham, 9 years ago

Patch needs improvement: set

Comments for improvement on the PR.

comment:7 by Tim Graham, 9 years ago

Patch needs improvement: unset

I update the PR per my comments.

comment:8 by Tim Graham, 9 years ago

Patch needs improvement: set

Simon still has concerns described on the pull request.

comment:9 by Simon Charette, 6 months ago

Owner: changed from nobody to Simon Charette
Status: newassigned

comment:10 by Simon Charette, 6 months ago

Updated patch that makes use of RETURNING on backends that support it and clear the attributes otherwise (allowing refresh_from_db to kick in on attribute access). Still needs a few tweaks.

comment:11 by Simon Charette, 6 months ago

Patch needs improvement: unset

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 3 weeks ago

In dc4ee99:

Refs #27222 -- Implemented BaseDatabaseOperations.return_insert_columns()/fetch_returned_insert_rows().

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 3 weeks ago

In 292b9e6f:

Refs #27222 -- Adapted RETURNING handling to be usable for UPDATE queries.

Renamed existing methods and abstractions used for INSERT … RETURNING
to be generic enough to be used in the context of UPDATEs as well.

This also consolidates SQL compliant implementations on
BaseDatabaseOperations.

comment:14 by Mariusz Felisiak, 5 days ago

Triage Stage: AcceptedReady for checkin

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 4 days ago

In 55a0073b:

Refs #27222 -- Refreshed GeneratedFields values on save() initiated update.

This required implementing UPDATE RETURNING machinery that heavily
borrows from the INSERT one.

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 4 days ago

Resolution: fixed
Status: assignedclosed

In 9468043:

Fixed #27222 -- Refreshed model field values assigned expressions on save().

Removed the can_return_columns_from_insert skip gates on existing
field_defaults tests to confirm the expected number of queries are
performed and that returning field overrides are respected.

comment:17 by Jacob Walls <jacobtylerwalls@…>, 35 hours ago

In e059bbe:

Refs #27222 -- Deduplicated db_returning fields in Model.save().

Follow-up to 94680437a45a71c70ca8bd2e68b72aa1e2eff337.

comment:18 by Jacob Walls <jacobtylerwalls@…>, 16 hours ago

In 4fcc2883:

Refs #27222 -- Restored Model.save()'s refreshing of db_returning fields even if a value is set.

The logic could likely be adjusted to assign the pre_save value in most cases
to avoid the database transit but it could break in subtle ways so it's not
worth the complexity it would require.

Regression in 94680437a45a71c70ca8bd2e68b72aa1e2eff337.

Co-authored-by: Tim Graham <timograham@…>

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