Implement a much more efficient approach to range queries. (#3831)

src/oscar/apps/catalogue/expressions.py

@@ -0,0 +1,34 @@
+from django.db.models.expressions import Subquery
+
+EXPAND_UPWARDS_CATEGORY_QUERY = """
+(SELECT "CATALOGUE_CATEGORY_JOIN"."id" FROM "catalogue_category" AS "CATALOGUE_CATEGORY_BASE"
+LEFT JOIN "catalogue_category" AS "CATALOGUE_CATEGORY_JOIN" ON (
+    "CATALOGUE_CATEGORY_BASE"."path" LIKE "CATALOGUE_CATEGORY_JOIN"."path" || '%%%%'
+    AND "CATALOGUE_CATEGORY_BASE"."depth" >= "CATALOGUE_CATEGORY_JOIN"."depth"
+)
+WHERE "CATALOGUE_CATEGORY_BASE"."id" IN (%(subquery)s))
+"""
+
+
+EXPAND_DOWNWARDS_CATEGORY_QUERY = """
+(SELECT "CATALOGUE_CATEGORY_JOIN"."id" FROM "catalogue_category" AS "CATALOGUE_CATEGORY_BASE"
+LEFT JOIN "catalogue_category" AS "CATALOGUE_CATEGORY_JOIN" ON (
+    "CATALOGUE_CATEGORY_JOIN"."path" LIKE "CATALOGUE_CATEGORY_BASE"."path" || '%%%%'
+    AND "CATALOGUE_CATEGORY_BASE"."depth" <= "CATALOGUE_CATEGORY_JOIN"."depth"
+)
+WHERE "CATALOGUE_CATEGORY_BASE"."id" IN (%(subquery)s))
+"""
+
+
+class ExpandUpwardsCategoryQueryset(Subquery):
+    template = EXPAND_UPWARDS_CATEGORY_QUERY
+
+    def as_sqlite(self, compiler, connection):
+        return super().as_sql(compiler, connection, self.template[1:-1])
+
+
+class ExpandDownwardsCategoryQueryset(Subquery):
+    template = EXPAND_DOWNWARDS_CATEGORY_QUERY
+
+    def as_sqlite(self, compiler, connection):
+        return super().as_sql(compiler, connection, self.template[1:-1])

src/oscar/apps/offer/abstract_models.py

@@ -19,6 +19,7 @@ from oscar.core.loading import (
 from oscar.models import fields
 from oscar.templatetags.currency_filters import currency
 
+ExpandDownwardsCategoryQueryset = get_class("catalogue.expressions", "ExpandDownwardsCategoryQueryset")
 ActiveOfferManager, RangeManager, BrowsableRangeManager \
     = get_classes('offer.managers', ['ActiveOfferManager', 'RangeManager', 'BrowsableRangeManager'])
 ZERO_DISCOUNT = get_class('offer.results', 'ZERO_DISCOUNT')
@@ -931,38 +932,30 @@ class AbstractRange(models.Model):
             )
 
         if self.included_categories.exists():
-            # build query to select all category subtrees.
-            category_filter = Q()
-            for path, depth in self.included_categories.values_list("path", "depth"):
-                category_filter |= Q(
-                    categories__depth__gte=depth, categories__path__startswith=path
-                )
-
-            # select all those product that are selected either by product class,
-            # category, or explicitly by included_products.
-            selected_products = Product.objects.annotate(
-                selected_categories=models.FilteredRelation(
-                    "categories", condition=category_filter
-                )
-            ).filter(
-                Q(product_class_id__in=self.classes.values("id"))
-                | Q(selected_categories__isnull=False)
-            ) | self.included_products.all()
+            expanded_range_categories = ExpandDownwardsCategoryQueryset(
+                self.included_categories.values("id")
+            )
+            selected_products = Product.objects.filter(
+                Q(categories__in=expanded_range_categories)
+                | Q(product_class__classes=self)
+                | Q(includes=self)
+                | Q(parent__categories__in=expanded_range_categories)
+                | Q(parent__product_class__classes=self)
+                | Q(parent__includes=self),
+                ~Q(excludes=self),
+                ~Q(parent__excludes=self)
+            )
         else:
             selected_products = Product.objects.filter(
-                product_class_id__in=self.classes.values("id")
-            ) | self.included_products.all()
-
-        # Include children of matching parents
-        selected_products = selected_products | Product.objects.filter(
-            parent__in=selected_products.filter(structure=Product.PARENT)
-        )
-
-        # now go and exclude all explicitly excluded products
-        excludes = self.excluded_products.values("id")
-        return selected_products.exclude(
-            Q(parent_id__in=excludes) | Q(id__in=excludes)
-        ).distinct()
+                Q(product_class__classes=self)
+                | Q(includes=self)
+                | Q(parent__product_class__classes=self)
+                | Q(parent__includes=self),
+                ~Q(excludes=self),
+                ~Q(parent__excludes=self)
+            )
+
+        return selected_products.distinct()
 
     @property
     def is_editable(self):

src/oscar/apps/offer/queryset.py

@@ -1,25 +1,8 @@
 from django.db import models
-from django.db.models import Exists, OuterRef
-
-
-def product_class_as_queryset(product):
-    "Returns a queryset with the product_classes of a product (only one)"
-    ProductClass = product._meta.get_field("product_class").related_model
-    return ProductClass.objects.filter(
-        pk__in=product.__class__.objects.filter(pk=product.pk)
-        .annotate(
-            _product_class_id=models.Case(
-                models.When(
-                    structure=product.CHILD, then=models.F("parent__product_class")
-                ),
-                models.When(
-                    structure__in=[product.PARENT, product.STANDALONE],
-                    then=models.F("product_class"),
-                ),
-            )
-        )
-        .values("_product_class_id")
-    )
+
+from oscar.core.loading import get_class
+
+ExpandUpwardsCategoryQueryset = get_class("catalogue.expressions", "ExpandUpwardsCategoryQueryset")
 
 
 class RangeQuerySet(models.query.QuerySet):
@@ -27,31 +10,55 @@ class RangeQuerySet(models.query.QuerySet):
     This queryset add ``contains_product`` which allows selecting the
     ranges that contain the product in question.
     """
-    def contains_product(self, product):
-        "Return ranges that contain ``product`` in a single query"
+
+    def _excluded_products_clause(self, product):
         if product.structure == product.CHILD:
-            return self._ranges_that_contain_product(
-                product.parent
-            ) | self._ranges_that_contain_product(product)
-        return self._ranges_that_contain_product(product)
-
-    def _ranges_that_contain_product(self, product):
-        Category = product.categories.model
-        included_in_subtree = product.categories.filter(
-            path__startswith=OuterRef("path")
-        )
-        category_tree = Category.objects.annotate(
-            is_included_in_subtree=Exists(included_in_subtree.values("id"))
-        ).filter(is_included_in_subtree=True)
+            # child products are excluded from a range if either they are
+            # excluded, or their parent.
+            return ~(
+                models.Q(excluded_products=product)
+                | models.Q(excluded_products__id=product.parent_id)
+            )
+        return ~models.Q(excluded_products=product)
 
+    def _included_products_clause(self, product):
+        if product.structure == product.CHILD:
+            # child products are included in a range if either they are
+            # included, or their parent is included
+            return models.Q(included_products=product) | models.Q(
+                included_products__id=product.parent_id
+            )
+        else:
+            return models.Q(included_products=product)
+
+    def _productclasses_clause(self, product):
+        if product.structure == product.CHILD:
+            # child products are included in a range if their parent is
+            # included in the range by means of their productclass.
+            return models.Q(classes__products__parent_id=product.parent_id)
+        return models.Q(classes__id=product.product_class_id)
+
+    def _get_category_ids(self, product):
+        if product.structure == product.CHILD:
+            # Since a child can not be in a catagory, it must be determined
+            # which category the parent is in
+            ProductCategory = product.productcategory_set.model
+            return ProductCategory.objects.filter(product_id=product.parent_id).values("category_id")
+
+        return product.categories.values("id")
+
+    def contains_product(self, product):
+        # the wide query is used to determine which ranges have includes_all_products
+        # turned on, we only need to look at explicit exclusions, the other
+        # mechanism for adding a product to a range don't need to be checked
         wide = self.filter(
-            ~models.Q(excluded_products=product), includes_all_products=True
+            self._excluded_products_clause(product), includes_all_products=True
         )
         narrow = self.filter(
-            ~models.Q(excluded_products=product),
-            models.Q(included_products=product)
-            | models.Q(included_categories__in=category_tree)
-            | models.Q(classes__in=product_class_as_queryset(product)),
+            self._excluded_products_clause(product),
+            self._included_products_clause(product)
+            | models.Q(included_categories__in=ExpandUpwardsCategoryQueryset(self._get_category_ids(product)))
+            | self._productclasses_clause(product),
             includes_all_products=False,
         )
         return wide | narrow

tests/integration/offer/test_range.py

@@ -295,13 +295,15 @@ class TestRangeQuerySet(TestCase):
     def test_contains_parent(self):
         ranges = models.Range.objects.contains_product(self.parent)
         self.assertEqual(
-            ranges.count(), 1, "Both ranges should contain the parent product"
+            ranges.count(), 1, "One range should contain the parent product"
         )
 
     def test_exclude_child(self):
         ranges = models.Range.objects.contains_product(self.child2)
         self.assertEqual(
-            ranges.count(), 1, "Only 1 range should contain the second child"
+            ranges.count(), 0,
+            "None of the ranges should contain the second child, because it"
+            " was excluded in the range that contains the parent."
         )
 
     def test_category(self):
@@ -329,10 +331,25 @@ class TestRangeQuerySet(TestCase):
             "The range containing the parent category of the parent product, should be selected",
         )
 
+        ranges = models.Range.objects.contains_product(self.child2)
+        self.assertEqual(
+            ranges.count(),
+            1,
+            "Since the parent category is part of the range, There should be 1 "
+            "range containing the child2 product, whose parent is in a subcategory",
+        )
+
         ranges = models.Range.objects.contains_product(self.child1)
         self.assertEqual(
             ranges.count(),
             3,
             "Since the parent category is part of the range, There should be 3 "
-            "ranges containing the child product, which is in a subcategory",
+            "ranges containing the child1 product, whose parent is in a subcategory",
+        )
+        cat_range.excluded_products.add(self.child2)
+        ranges = models.Range.objects.contains_product(self.child2)
+        self.assertEqual(
+            ranges.count(),
+            0,
+            "No ranges should contain child2 after explicitly removing it from the only range that contained it",
         )