Opened 3 years ago

Closed 3 years ago

#32563 closed Bug (invalid)

Cannot override database used with RelatedManager

Reported by: Lucas Gruber Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: RelatedManager mullti databases
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Lucas Gruber)

I suppose an app with models:

############################
class Blog(models.Model):
    title = models.CharField(max_length=100)

class Person(models.Model):
    name = models.CharField(max_length=100)
    subscribed_blogs = models.ManyToManyField(Blog, related_name="subscribers", through="Subscription")
    
class Subscription(models.Model):
    person = models.ForeignKey(Person, related_name="subscriptions")
    blog = models.ForeignKey(Blog, related_name="subscriptions")
############################

In thes case we are using multiple databases and we need to create custom migration, we have to write something like

############################
def custom_migrations(apps, schema_editor):
    db_alias = schema_editor.connection.alias
    
    person = Person.objects.using(db_alias).get(pk=1)
    blog = Blog.objects.using(db_alias).get(pk=1)
    blog.subscribers.set([person])

    blog.save(using=db_alias)

############################

The line blog.subscribers.set(...) does not permit to add parameter for overriding database to use.
The source code for this function is in django.db.models.fields.related_descriptors when we can see:

[...]
        def set(self, objs, *, clear=False, through_defaults=None):
            # Force evaluation of `objs` in case it's a queryset whose value
            # could be affected by `manager.clear()`. Refs #19816.
            objs = tuple(objs)

            db = router.db_for_write(self.through, instance=self.instance)
            with transaction.atomic(using=db, savepoint=False):
                if clear:
                    self.clear()
                    self.add(*objs, through_defaults=through_defaults)
                else:
                    old_ids = set(self.using(db).values_list(self.target_field.target_field.attname, flat=True))

                    new_objs = []
                    for obj in objs:
                        fk_val = (
                            self.target_field.get_foreign_related_value(obj)[0]
                            if isinstance(obj, self.model) else obj
                        )
                        if fk_val in old_ids:
                            old_ids.remove(fk_val)
                        else:
                            new_objs.append(obj)

                    self.remove(*old_ids)
                    self.add(*new_objs, through_defaults=through_defaults)
[...]

Code always calls database router, but in migration process, the router can not find the appropriate database because we just use without request :

python manage.py migrate --database db2

I noticed that all the methods of RelatedManager directly call the router object to find the database while the Manager objects always exploits the possibility of overriding the database with the call to using() on the QuerySet or to pass parameter using=db for save model method for example.

I think we should use the same mechanism which is used in the Model class with the save() method :

    def save(self, force_insert=False, force_update=False, using=None,
             update_fields=None):
        [...]
        using = using or router.db_for_write(self.__class__, instance=self)
        [...]

Thus, we will be able to override the database at the time of the save.

Thank you in advance for your answer

Change History (8)

comment:1 by Lucas Gruber, 3 years ago

Description: modified (diff)

comment:2 by Lucas Gruber, 3 years ago

Description: modified (diff)

comment:3 by Carlton Gibson, 3 years ago

Resolution: invalid
Status: newclosed

Hi, sorry there's not an issue report in there. Please see TicketClosingReasons/UseSupportChannels.
(This looks like the sort of thing StackOverflow is good at.)

comment:4 by Lucas Gruber, 3 years ago

Hello,

I do not understand your response. I do not ask for helping...
All method and all other objects in database layer allow to override database because else, it not possible to use custom migration with multiple databases.
Only one case does not allows this functionality : it is this case with RelatedManager...

There is no solution for using custom migration with database router in the current version of Django. I have overridden this bug currently by exploiting a static variable that I manually modify in the database migration to force my router to use the database without relying on the sites application.

So yes, i think it is a bug and there is no a clean solution with actual code.

Please could you reconsider closing this ticket ?

comment:5 by Lucas Gruber, 3 years ago

Resolution: invalid
Status: closednew

comment:6 by Lucas Gruber, 3 years ago

Description: modified (diff)

comment:7 by Lucas Gruber, 3 years ago

Description: modified (diff)

comment:8 by Mariusz Felisiak, 3 years ago

Resolution: invalid
Status: newclosed

Lucas, I'm not exactly sure how you would like to use using in set(). There is no need to specify a database alias for set() because it uses db_alias from the instance. As far as I'm aware that's the only database that makes sense in this case. Cross-database foreign keys and m2m fields are not supported in Django, see #12540.

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