Opened 12 years ago

Closed 9 years ago

Last modified 9 years ago

#17520 closed Bug (fixed)

write queries on related managers use the db_for_read database

Reported by: Tobias McNulty Owned by: Sławek Ehlert
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

If you execute a write query (update() or delete()) on a queryset obtained from a related manager, it appears to use the db_for_read database instead of the db_for_write database for that query.

The attached patch reproduces the issue on trunk. It also exists in 1.3.1.

Attachments (3)

related_writes_use_write_db.diff (2.4 KB ) - added by Tobias McNulty 12 years ago.
related_writes_use_write_db_v2.diff (2.4 KB ) - added by Sławek Ehlert 11 years ago.
slightly modified version for better debugging
related_writes_use_write_db_m2m.diff (4.3 KB ) - added by Sławek Ehlert 11 years ago.
test case for a ManyToMany scenario

Download all attachments as: .zip

Change History (13)

by Tobias McNulty, 12 years ago

comment:1 by Tobias McNulty, 12 years ago

The issue appears to be that query._db gets set when the queryset is created from a related manager. As such, when the QuerySet.db property checks to see what write db to use, it assumes using() has been called manually and does not call db_for_write on the router as it should.

comment:2 by Tobias McNulty, 12 years ago

Patch needs improvement: set

The patch only has a test currently (not sure why that got unset, I didn't uncheck anything).

comment:3 by Aymeric Augustin, 12 years ago

Triage Stage: UnreviewedAccepted

Indeed, the test case looks valid and fails.

comment:4 by Sławek Ehlert, 11 years ago

Owner: changed from nobody to Sławek Ehlert
Status: newassigned

comment:5 by Sławek Ehlert, 11 years ago

The problem seems to be very subtle. The thing is that the update method on queryset is using self.db which uses self._db to determine the database. When we have a related manager, the queryset from it comes with "polluted" _db (because of this ).

When i tried a simplest approach with cleaning the _db after getting the qs like this:

if self._db is None:
    qs._db = None

it turns out that there's some feature tests that gets broken. For example this (which I think isn't documented anywhere). Will try to propose something different.

by Sławek Ehlert, 11 years ago

slightly modified version for better debugging

by Sławek Ehlert, 11 years ago

test case for a ManyToMany scenario

comment:7 by Sławek Ehlert, 11 years ago

Patch needs improvement: unset

I attached a newer version of tobias patch.

I also added a patch with a test case that shows that the ManyToMany scenario is also broken (you can check it here also)

comment:8 by Sławek Ehlert, 11 years ago

Patch needs improvement: set

comment:9 by Tim Graham, 9 years ago

Resolution: fixed
Status: assignedclosed

Fixed in Django 1.7 with #13724 / 9595183d03cfd0d94ae2dd506a3d2b86cf5c74a7.

comment:10 by Tobias McNulty, 9 years ago

Many thanks!

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