From 33e46ebd533dd964b9cb7cfaae833b6b891f335f Mon Sep 17 00:00:00 2001 From: Skia Date: Mon, 6 Jul 2020 15:18:42 +0200 Subject: [PATCH] managers: avoid querying too much tables when not needed When calling `select_related()` with an empty list of arguments [1], Django will try to prefetch some data by doing some first level joints with the related classes. This can lead to obvious negative performance impact, but this also breaks some workarounds for having inheritance for foreign keys [2], as those solutions rely on lazy evaluation of the related object. [1]: https://github.com/django/django/blob/a4e6030904df63b3f10aa0729b86dc6942b0458e/django/db/models/query.py#L1051 Only passing an explicit `None` to `select_related` will disable the magic. [2]: https://github.com/jazzband/django-model-utils/issues/11 As examples, here are the generated SQL requests in InheritanceManagerRelatedTests.test_get_method_with_select_subclasses_check_for_useless_join: * without this fix, without adding `.select_related(None)` ```sql SELECT "tests_inheritancemanagertestparent"."id", "tests_inheritancemanagertestparent"."non_related_field_using_descriptor", "tests_inheritancemanagertestparent"."related_id", "tests_inheritancemanagertestparent"."normal_field", "tests_inheritancemanagertestparent"."related_self_id", "tests_inheritancemanagertestchild4"."other_onetoone_id", "tests_inheritancemanagertestchild4"."parent_ptr_id", T3."id", T3."non_related_field_using_descriptor", T3."related_id", T3."normal_field", T3."related_self_id" FROM "tests_inheritancemanagertestchild4" INNER JOIN "tests_inheritancemanagertestparent" ON ("tests_inheritancemanagertestchild4"."parent_ptr_id" = "tests_inheritancemanagertestparent"."id") INNER JOIN "tests_inheritancemanagertestparent" T3 ON ("tests_inheritancemanagertestchild4"."other_onetoone_id" = T3."id") WHERE "tests_inheritancemanagertestchild4"."parent_ptr_id" = 191 ``` * with either the fix, or by adding `.select_related(None)` after `.select_subclasses()` ```sql SELECT "tests_inheritancemanagertestparent"."id", "tests_inheritancemanagertestparent"."non_related_field_using_descriptor", "tests_inheritancemanagertestparent"."related_id", "tests_inheritancemanagertestparent"."normal_field", "tests_inheritancemanagertestparent"."related_self_id", "tests_inheritancemanagertestchild4"."other_onetoone_id", "tests_inheritancemanagertestchild4"."parent_ptr_id" FROM "tests_inheritancemanagertestchild4" INNER JOIN "tests_inheritancemanagertestparent" ON ("tests_inheritancemanagertestchild4"."parent_ptr_id" = "tests_inheritancemanagertestparent"."id") WHERE "tests_inheritancemanagertestchild4"."parent_ptr_id" = 191 ``` --- CHANGES.rst | 1 + model_utils/managers.py | 5 ++++- tests/test_managers/test_inheritance_manager.py | 8 ++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 6f5ecd8..cd1bb7d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -3,6 +3,7 @@ CHANGES 4.0.1 (unreleased) ------------------ +- Update InheritanceQuerySetMixin to avoid querying too much tables - TimeStampedModel now automatically adds 'modified' field as an update_fields parameter even if it is forgotten while using save() - `FieldTracker` now marks fields as not changed after `refresh_from_db` diff --git a/model_utils/managers.py b/model_utils/managers.py index 105794b..3725eab 100644 --- a/model_utils/managers.py +++ b/model_utils/managers.py @@ -75,7 +75,10 @@ class InheritanceQuerySetMixin: # workaround https://code.djangoproject.com/ticket/16855 previous_select_related = self.query.select_related - new_qs = self.select_related(*subclasses) + if subclasses: + new_qs = self.select_related(*subclasses) + else: + new_qs = self previous_is_dict = isinstance(previous_select_related, dict) new_is_dict = isinstance(new_qs.query.select_related, dict) if previous_is_dict and new_is_dict: diff --git a/tests/test_managers/test_inheritance_manager.py b/tests/test_managers/test_inheritance_manager.py index 2481b5c..43ce50b 100644 --- a/tests/test_managers/test_inheritance_manager.py +++ b/tests/test_managers/test_inheritance_manager.py @@ -487,6 +487,14 @@ class InheritanceManagerRelatedTests(InheritanceManagerTests): id=self.child1.id), self.child1) + def test_get_method_with_select_subclasses_check_for_useless_join(self): + child4 = InheritanceManagerTestChild4.objects.create(related=self.related, other_onetoone=self.child1) + self.assertEqual( + str(InheritanceManagerTestChild4.objects.select_subclasses().filter( + id=child4.id).query), + str(InheritanceManagerTestChild4.objects.select_subclasses().select_related(None).filter( + id=child4.id).query)) + def test_annotate_with_select_subclasses(self): qs = InheritanceManagerTestParent.objects.select_subclasses().annotate( models.Count('id'))