Code

#19096 closed Cleanup/optimization (fixed)

SQLInsertCompiler and DatabaseOperations.return_insert_id for non standard backends

Reported by: manfre Owned by: nobody
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I ran in to a problem with SQLInsertCompiler while trying to allow django-mssql to have can_return_id_from_insert = True. The problem is that SQLInsertCompiler requires that an SQL fragment (returned by ops.return_insert_id) is appended to the end of the insert statement, but the fragment must also accept the full quoted table and column name.

https://github.com/django/django/blob/master/django/db/models/sql/compiler.py#L900

The problem for mssql, is that the OUTPUT clause doesn't conform to these two requirements and is better served by mangling the SQL query in its own SQLInsertCompiler. To make this possible (in a backwards compatible way), the output of return_insert_id needs to be checked and ignored if r_fmt is None.

Attachments (0)

Change History (3)

comment:1 Changed 19 months ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

RETURNING is an extension of the SQL standard. We can't make assumptions about how the various databases implement it.

Adding a if r_fmt is None: here looks harmless.

I'm in favor of helping 3rd party database adapters when we can.

comment:3 Changed 19 months ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from new to closed

In c2150d4d2c5342488e474825c67dd3210fafc0e7:

Fixed #19096 -- Made can_return_id_from_insert more extendable

RETURNING is an extension of the SQL standard, which is not implemented
the same by all databases. Allow DatabaseOperations.return_insert_id to
return a None to allow for other 3rd party backends with a different
implementation.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.