order_with_respect_to uses incorrect database alias

[Damian Posener]

PR: ​https://github.com/django/django/pull/15409

When calling set_RELATED_order on a model that uses order_with_respect_to, if a DB router exists that routes that model to a non-default database, the routing is ignored, causing a datbase error to occur.

I've tracked this down to the method_set_order function here:
​https://github.com/django/django/blob/3.2.12/django/db/models/base.py#L2117

This function updates using=None to using=DEFAULT_DB_ALIAS, which then overrides any database routing set up on the model manager. It should just pass through using=None so that the manager can deal with determining the correct connection to use.

I believe this got inadvertently broken in #30106 - previously the using parameter was actually being ignored, but when that got fixed it caused this new error. :)

[Mariusz Felisiak]

Thanks for the report.

[Simon Charette]

Interestingly this means that the code prior to 8d2dcc47cd7f6069a1fa2c5b67a329cec5846b7e was broken in another way. It use to open a transaction against the default database but the following chunked updates were performed against whatever db_for_write returned afterwards making them non-atomic when performed against the non-default database.

This makes me realize that bulk_update doesn't set self._for_write = True before accessing self.db (like all other create and update queryset methods do) which means the aforementioned issue is affects this function as well so I'll file another issue for it and assign it to myself.

You patch looks great Damian! It'd be even better if you write a test that ensures the updates are performed against the right database to prevent future regressions. I suggest you grep the test directory for override_settings(DATABASE_ROUTERS to get an idea of how you can route queries to the other test database and use assertNumQueries(using='other') to ensure it was properly routed.

[Mariusz Felisiak <felisiak.mariusz@…>]

In d35ce682:

Fixed #33506 -- Made QuerySet.bulk_update() perform atomic writes against write database.

The lack of _for_write = True assignment in bulk_update prior to
accessing self.db resulted in the db_for_read database being used to
wrap batched UPDATEs in a transaction.

Also tweaked the batch queryset creation to also ensure they are
executed against the same database as the opened transaction under all
circumstances.

Refs #23646, #33501.

[Damian Posener]

Replying to Simon Charette:

You patch looks great Damian! It'd be even better if you write a test that ensures the updates are performed against the right database to prevent future regressions. I suggest you grep the test directory for override_settings(DATABASE_ROUTERS to get an idea of how you can route queries to the other test database and use assertNumQueries(using='other') to ensure it was properly routed.

Thanks for the pointer, I've managed to sort out a regression test and have added it to my PR above. 👍

[Mariusz Felisiak <felisiak.mariusz@…>]

In 09e499a3:

Fixed #33501 -- Made order_with_respect_to respect database routers.