[Django] #27222: Refresh expressions on save

[Django]

#27222: Refresh expressions on save
----------------------------------------------+--------------------
Reporter: holvianssi | Owner: nobody
Type: New feature | Status: new
Component: Database layer (models, ORM) | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
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
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/27222>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#27222: Refresh expressions on save

-------------------------------------+-------------------------------------

Reporter: holvianssi | Owner: nobody
Type: New feature | Status: new

Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by holvianssi):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0

Old description:

> 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
> }}}

New description:

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.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:1>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------

Reporter: holvianssi | Owner: nobody
Type: New feature | Status: new

Component: Database layer | Version: 1.10
(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 timgraham):

* stage: Unreviewed => Accepted

Comment:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:2>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------

Reporter: holvianssi | Owner: nobody
Type: New feature | Status: new

Component: Database layer | Version: 1.10
(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 charettes):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/7244 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:3>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------

Reporter: holvianssi | Owner: nobody
Type: New feature | Status: new

Component: Database layer | Version: 1.10
(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

-------------------------------------+-------------------------------------

Comment (by charettes):

This might also be related to #21454 or at least to the implementation
proposed in it's [https://github.com/django/django/pull/5904 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.

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

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------

Reporter: holvianssi | Owner: nobody
Type: New feature | Status: new

Component: Database layer | Version: 1.10
(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

-------------------------------------+-------------------------------------

Comment (by holvianssi):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:5>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------

Reporter: holvianssi | Owner: nobody
Type: New feature | Status: new

Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by timgraham):

* needs_better_patch: 0 => 1

Comment:

Comments for improvement on the PR.

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

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------

Reporter: holvianssi | Owner: nobody
Type: New feature | Status: new

Component: Database layer | Version: 1.10
(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 Tim Graham):

* needs_better_patch: 1 => 0

Comment:

I update the PR per my comments.

--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:7>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------

Reporter: holvianssi | Owner: nobody
Type: New feature | Status: new

Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* needs_better_patch: 0 => 1

Comment:

Simon still has concerns described on the pull request.

--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:8>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------

Reporter: holvianssi | Owner: Simon
| Charette
Type: New feature | Status: assigned

Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* owner: nobody => Simon Charette
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:9>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------
Reporter: holvianssi | Owner: Simon
| Charette
Type: New feature | Status: assigned
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Comment (by Simon Charette):

Updated [https://github.com/django/django/pull/19285 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:10>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------
Reporter: holvianssi | Owner: Simon
| Charette
Type: New feature | Status: assigned
Component: Database layer | Version: 1.10
(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 Simon Charette):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:11>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------
Reporter: holvianssi | Owner: Simon
| Charette
Type: New feature | Status: assigned
Component: Database layer | Version: 1.10
(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

-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"dc4ee9915228238bd24ce67645504f65eaf2f1fd" dc4ee99]:
{{{#!CommitTicketReference repository=""
revision="dc4ee9915228238bd24ce67645504f65eaf2f1fd"
Refs #27222 -- Implemented
BaseDatabaseOperations.return_insert_columns()/fetch_returned_insert_rows().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:12>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------
Reporter: holvianssi | Owner: Simon
| Charette
Type: New feature | Status: assigned
Component: Database layer | Version: 1.10
(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

-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"292b9e6fe8f23491680d9cc60f328562e2b1c823" 292b9e6f]:
{{{#!CommitTicketReference repository=""
revision="292b9e6fe8f23491680d9cc60f328562e2b1c823"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:13>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------
Reporter: holvianssi | Owner: Simon
| Charette
Type: New feature | Status: assigned
Component: Database layer | Version: 1.10
(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 Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:14>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------
Reporter: holvianssi | Owner: Simon
| Charette
Type: New feature | Status: assigned
Component: Database layer | Version: 1.10
(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

-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"55a0073b3beb9de8f7c1f7c44a7d0bc10126c841" 55a0073b]:
{{{#!CommitTicketReference repository=""
revision="55a0073b3beb9de8f7c1f7c44a7d0bc10126c841"
Refs #27222 -- Refreshed GeneratedFields values on save() initiated
update.

This required implementing UPDATE RETURNING machinery that heavily
borrows from the INSERT one.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:15>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------
Reporter: holvianssi | Owner: Simon
| Charette

Type: New feature | Status: closed

Component: Database layer | Version: 1.10
(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 Mariusz Felisiak <felisiak.mariusz@…>):

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

Comment:

In [changeset:"94680437a45a71c70ca8bd2e68b72aa1e2eff337" 9468043]:
{{{#!CommitTicketReference repository=""
revision="94680437a45a71c70ca8bd2e68b72aa1e2eff337"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:16>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------
Reporter: holvianssi | Owner: Simon
| Charette
Type: New feature | Status: closed
Component: Database layer | Version: 1.10
(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

-------------------------------------+-------------------------------------
Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"e059bbec96ed85d5f4d63893d687c8af966430c8" e059bbe]:
{{{#!CommitTicketReference repository=""
revision="e059bbec96ed85d5f4d63893d687c8af966430c8"
Refs #27222 -- Deduplicated db_returning fields in Model.save().

Follow-up to 94680437a45a71c70ca8bd2e68b72aa1e2eff337.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:17>

[Django]

#27222: Refresh fields that are expressions after Model.save()
-------------------------------------+-------------------------------------
Reporter: holvianssi | Owner: Simon
| Charette
Type: New feature | Status: closed
Component: Database layer | Version: 1.10
(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

-------------------------------------+-------------------------------------
Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"4fcc2883faa8e33152d9e45744354cff35636975" 4fcc2883]:
{{{#!CommitTicketReference repository=""
revision="4fcc2883faa8e33152d9e45744354cff35636975"
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 <timog...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27222#comment:18>