Opened 5 years ago

Closed 5 years ago

#17467 closed Bug (fixed)

compare id of Site object with SITE_ID before updating (in create_test_db)

Reported by: kallous@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Release blocker Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

MSSQL Server cannot update identity columns, but 1.3.1 introduced a change in django.db.backends.creation at line 385 which ensures the Site object instance id matches the SITE_ID in <project>.settings. This is just a recommendation to test the value before updating it to prevent unnecessary complications.

Attachments (1)

django_db_create_test_db.diff (726 bytes) - added by kallous@… 5 years ago.

Download all attachments as: .zip

Change History (7)

Changed 5 years ago by kallous@…

comment:1 Changed 5 years ago by Michael Manfre

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Michael Manfre
Patch needs improvement: unset
Status: newassigned
Triage Stage: UnreviewedAccepted

#15573 was the breaking change (Oracle test support was fixed at the expense of MSSQL).

comment:2 Changed 5 years ago by Aymeric Augustin

See also #17415.

comment:3 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Given the history that led to the current code (around 10 tickets), this isn't really an easy picking.

comment:4 Changed 5 years ago by Michael Manfre

Owner: changed from Michael Manfre to nobody
Status: assignednew

I added a temporary fix to django-mssql (https://code.google.com/p/django-mssql/source/detail?r=08bba3453814d7f00ee93e2c8077a82a060265b7), which overrides create_test_db and removes the offending update. The change to django-mssql is a temporary solution to allow running unit tests and does not remove the need to fix this in django.

comment:5 Changed 5 years ago by Aymeric Augustin

Severity: NormalRelease blocker
Triage Stage: AcceptedDesign decision needed

Currently, Django allows forcing the primary key of an object — that's what #17415 is about — so, even if we rollback the offending code, it's a limitation of django-mssql.

I'm marking this as a release blocker, because it is a regression from the previous version, and it's a blocker for your (legitimate) use case.

But I really can't say what's the best solution. We're stuck by SITE_ID, a mandatory setting.

comment:6 Changed 5 years ago by Aymeric Augustin

Resolution: fixed
Status: newclosed

Sorry, I was confused in my previous comments. Like I said, there's a lot of history :)

The code you're suggesting to patch was removed at r16868 in trunk and at r16869 in 1.3.X. Django now ensures that the id of the default site object is 1 without updating the primary key; it creates the object directly with the proper id. If I understand correctly, that should fix your problem.

I just edited that code, here's what it looks like now on trunk and on 1.3.X.

Thanks for the report, and feel free to reopen this ticket if the issue persists! (I don't have access to a MSSQL database to validate the current code.)

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