Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#18347 closed Bug (fixed)

Unit tests contain raw inserts to an identity column

Reported by: Michael Manfre Owned by: Anssi Kääriäinen
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.

Change History (10)

comment:1 Changed 4 years ago by Michael 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 4 years ago by Aymeric Augustin

Triage Stage: UnreviewedAccepted

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 4 years ago by Anssi Kääriäinen

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 4 years ago by Michael Manfre

Tests pass with your patch.

comment:5 Changed 4 years ago by Michael Manfre

Owner: changed from Michael Manfre to Anssi Kääriäinen

Passing ownership because your patch works for django-mssql

comment:6 Changed 4 years ago by Jan Bednařík

Triage Stage: AcceptedReady for checkin

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

comment:7 Changed 4 years ago by Aymeric Augustin

The patch looks good to me too.

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

Resolution: fixed
Status: newclosed

In 71e14cf3aa024496adcb23e83ddf13a7c5ddeb32:

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

comment:9 Changed 4 years 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 4 years ago by Preston Holmes <preston@…>

In 9979c12f4a13a707fbfdd01f1694864fa53b6e6a:

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

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