Code

Opened 2 years ago

Closed 18 months ago

Last modified 17 months ago

#18347 closed Bug (fixed)

Unit tests contain raw inserts to an identity column

Reported by: manfre Owned by: akaariai
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: identity insert,mssql
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Two unit tests in transactions_regress.TestTransactionClosing fail for MSSQL because of raw inserts in to an identity column. MSSQL and potentially other 3rd party backends require actions to allow this type of insert.

Attachments (0)

Change History (10)

comment:1 Changed 2 years ago by manfre

  • Easy pickings set
  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Pull request https://github.com/manfre/django/commit/06d8aee7edc1ff2862ad8c64d1097090df46fdfa

Added enable_identity_insert, disable_identity_insert, and
contextmanager identity_insert_enabled to DatabaseOperations for
database backends that need to take special actions to allow
identity inserts.

All raw insert statements that set a value in to the identity field
should include these guards to allow all database backends to
function.

Implementation followed the paradigm used by enable/disable constraints.

comment:2 Changed 23 months ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

The patch looks pretty good to me.

However, wouldn't it be just as easy to not include the primary key in the inserted values in these two tests? I wouldn't like to expand the database backend API more than necessary.

comment:3 Changed 23 months ago by akaariai

An alternate patch which just removes inserts into the primary key field. Would this work for mssql? If not, then adding a new model without autofield primary key is still one possibility.

https://github.com/akaariai/django/tree/ticket_18347

comment:4 Changed 23 months ago by manfre

Tests pass with your patch.

comment:5 Changed 19 months ago by manfre

  • Owner changed from manfre to akaariai

Passing ownership because your patch works for django-mssql

comment:6 Changed 18 months ago by Architekt

  • Triage Stage changed from Accepted to Ready for checkin

Patch from akaariai works, tests pass also for PostgreSQL and SQLite3 (if test is not skipped).

comment:7 Changed 18 months ago by aaugustin

The patch looks good to me too.

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

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

In 71e14cf3aa024496adcb23e83ddf13a7c5ddeb32:

Fixed #18347 -- Removed autofield raw SQL inserts from tests

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

In a1fd9555f2cc32d10cdbc4aa71f86bac50ffd9c1:

[1.5.x] Fixed #18347 -- Removed autofield raw SQL inserts from tests

Backpatch of 71e14cf3aa024496adcb23e83ddf13a7c5ddeb32

comment:10 Changed 17 months ago by Preston Holmes <preston@…>

In 9979c12f4a13a707fbfdd01f1694864fa53b6e6a:

Fixed #18347 -- Removed autofield raw SQL inserts from tests

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.