[Django] #35974: create_reverse_many_to_one_manager: use router.allow_relation instead of comparing objects' _state.db in RelatedManager.add

14 views
Skip to first unread message

Django

unread,
Dec 5, 2024, 6:41:44 AM12/5/24
to django-...@googlegroups.com
#35974: create_reverse_many_to_one_manager: use router.allow_relation instead of
comparing objects' _state.db in RelatedManager.add
-------------------------------------+-------------------------------------
Reporter: Laurent Szyster | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 4.2 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In the `create_reverse_many_to_one_manager` function, the
`RelatedManager.add` method will raise an `ValueError` if one of the
related objects to add to the many-to-many relation set has not been
loaded from the same database connection as the instance to which it is
related.

{{{
if bulk:
pks = []
for obj in objs:
check_and_update_obj(obj)
if obj._state.adding or obj._state.db != db:
raise ValueError(
"%r instance isn't saved. Use bulk=False or
save "
"the object first." % obj
)
pks.append(obj.pk)
self.model._base_manager.using(db).filter(pk__in=pks).update(
**{
self.field.name: self.instance,
}
)
}}}

See:
https://github.com/django/django/blob/39cf3c63f3228a04f101f3e62c75a6aae7c6ef0f/django/db/models/fields/related_descriptors.py#L767-L776

This implies that an object loaded from a read-only connection to a
database cannot be related to an instance created using a distinct write-
enabled connection to the same database.

For instance, assume the following Database Router:

{{{
class DatabaseRouter:
def db_for_read(self, model, **hints):
return "read-only"

def db_for_write(self, model, **hints):
return "default"

def allow_relation(self, value, instance, **hints):
if instance._state.db == 'default' and value._state.db in
{'default', 'read-only'}:
return True

return None

def allow_migrate(self, db, app_label, model_name=None, **hints):
return True
}}}

This router will dispatch read queries to a "read-only" database
connection and write queries to the "default" database connection. Such
setup is intended to limit the load on the writer node of a database
cluster.

But for such setup to work the router's `allow_relation` method should be
used instead of comparing the database connection used to load the related
value with the database to write the instance to which we want to add a
relation.

Line 771 of `related_descriptor.py`:

{{{
if obj._state.adding or obj._state.db != db:
}}}

Should be replaced with:

{{{
if obj._state.adding or not
(router.allow_relation(obj, self.instance) or obj._state.db == db):
}}}

See: https://docs.djangoproject.com/en/4.2/topics/db/multi-
db/#allow_relation
--
Ticket URL: <https://code.djangoproject.com/ticket/35974>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Dec 5, 2024, 7:03:37 AM12/5/24
to django-...@googlegroups.com
#35974: create_reverse_many_to_one_manager: use router.allow_relation instead of
comparing objects' _state.db in RelatedManager.add
-------------------------------------+-------------------------------------
Reporter: Laurent Szyster | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Laurent Szyster):

Note that the database router's `allow_relation` method is used elsewhere
to test if two objects can be related:

-
https://github.com/django/django/blob/39cf3c63f3228a04f101f3e62c75a6aae7c6ef0f/django/db/models/fields/related_descriptors.py#L284
-
https://github.com/django/django/blob/39cf3c63f3228a04f101f3e62c75a6aae7c6ef0f/django/db/models/fields/related_descriptors.py#L547
-
https://github.com/django/django/blob/39cf3c63f3228a04f101f3e62c75a6aae7c6ef0f/django/db/models/fields/related_descriptors.py#L1312
--
Ticket URL: <https://code.djangoproject.com/ticket/35974#comment:1>
Reply all
Reply to author
Forward
0 new messages