Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#29444 closed Cleanup/optimization (fixed)

Allow fields to be part of the RETURNING clause during INSERT

Reported by: Johannes Maron Owned by: Johannes Maron
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: django, db, returning, default, model, field
Cc: info@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Dependency for #27452

The SQL RETURNING statement is currently used for inserts of new objects. Mainly it is responsible to return the primary key in a single query.

That's great but currently in the postgres database backend this value is hard coded to only the primary key. I would like to propose to make this configurable.

There are plenty of uses cases for such a feature ultimately even enabling database defaults, but I would suggest to keep it very simple for now.

I would suggest to simply allow model fields to specify if they should be included in the returning statement or not. The default being not (except PK) to maintain the current behavior.

The implementation as well as the related code lines have been already discussed here:
​https://github.com/django/django/pull/7525#issuecomment-354269077

I highly suggest to keep this a Postgres ONLY feature for now. That way we can see what sticks and consider extending this functionality into django.db in the future after we have seen how it's being used and gathered feedback.

Attachments (1)

refs-29444-oracle-part.diff​ (8.2 KB ) - added by Mariusz Felisiak 5 years ago.

Download all attachments as: .zip

Change History (29)

comment:1 by Johannes Maron, 6 years ago

Version: 2.0 → master

comment:2 by Johannes Maron, 6 years ago

Owner: set to Johannes Maron
Status: new → assigned

comment:3 by Johannes Maron, 6 years ago

Has patch: set
Version 0, edited 6 years ago by Johannes Maron (next)

comment:4 by Johannes Maron, 6 years ago

Patch needs improvement: set
Triage Stage: Unreviewed → Accepted

comment:5 by Johannes Maron, 6 years ago

Patch needs improvement: unset

comment:6 by Tim Graham, 6 years ago

Patch needs improvement: set

comment:7 by Johannes Maron, 6 years ago

Patch needs improvement: unset
Type: New feature → Cleanup/optimization

comment:8 by Tim Graham, 6 years ago

Component: contrib.postgres → Database layer (models, ORM)
Patch needs improvement: set
Summary: Allow modification of RETURNING values in django.contrib.postgres → Allow fields to be part of the RETURNING clause during INSERT

A number of Oracle test failures remain.

comment:9 by Johannes Maron, 6 years ago

Patch needs improvement: unset

I fixed a couple of things. Nothing fails for me now locally that doesn't fail on master. Can you trigger it again?

comment:10 by Tim Graham <timograham@…>, 6 years ago

In b131f9c7:

Refs #29444 -- Renamed DatabaseFeatures.can_return_id* to be generic for other columns.

comment:11 by Tim Graham, 6 years ago

Patch needs improvement: set

You should be able to trigger the Oracle build yourself as described on wiki:Jenkins. I ran the tests locally and put the failures in a PR comment.

comment:12 by Johannes Maron, 6 years ago

Patch needs improvement: unset

comment:13 by Carlton Gibson, 5 years ago

Needs tests: set

comment:14 by Johannes Maron, 5 years ago

Needs tests: unset

comment:15 by Mariusz Felisiak, 5 years ago

Patch needs improvement: set

comment:16 by Johannes Maron, 5 years ago

Patch needs improvement: unset

comment:17 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In bc91f27a:

Refs #29444 -- Added support for fetching a returned non-integer insert values on Oracle.

This is currently not actively used, since the ORM will ask the
SQL compiler to only return auto fields.

comment:18 by Mariusz Felisiak, 5 years ago

Patch needs improvement: set

comment:19 by Johannes Maron, 5 years ago

Patch needs improvement: unset

comment:20 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 003bb34b:

Refs #29444 -- Made db.backends.oracle.utils.InsertVar use str as default.

comment:21 by Mariusz Felisiak, 5 years ago

Triage Stage: Accepted → Ready for checkin

comment:22 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 736e7d44:

Refs #29444 -- Fixed DateField constructor in db.backends.oracle.utils.InsertVar.

comment:23 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 7254f113:

Refs #29444 -- Allowed returning multiple fields from INSERT statements on PostgreSQL.

Thanks Florian Apolloner, Tim Graham, Simon Charette, Nick Pope, and
Mariusz Felisiak for reviews.

by Mariusz Felisiak, 5 years ago

Attachment: refs-29444-oracle-part.diff​ added

comment:24 by Mariusz Felisiak, 5 years ago

Has patch: unset
Triage Stage: Ready for checkin → Accepted

This ticket is doable on Oracle with cx_Oracle 6.0+ that's why I'm leaving it open. First attempt of fixing this on Oracle was a part of the original PR (see a diff).

Last edited 5 years ago by Mariusz Felisiak (previous) (diff)

comment:25 by Mariusz Felisiak, 5 years ago

Has patch: set
Patch needs improvement: set

​PR with adding support on Oracle

comment:26 by Johannes Maron, 5 years ago

Patch needs improvement: unset

comment:27 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: → fixed
Status: assigned → closed

In b31e6387:

Fixed #29444 -- Allowed returning multiple fields from INSERT statements on Oracle.

comment:28 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 0110436:

Refs #29444 -- Removed redundant DatabaseFeatures.can_return_multiple_columns_from_insert.

Unnecessary since b31e63879eb5d9717e9f890401f7222e4f00c910.

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