launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/gSCFPS-optimise into lp:launchpad.

Commit message:
Optimise SpecificationSet.getStatusCountsForProductSeries, fixing Product:+series timeouts.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/gSCFPS-optimise/+merge/306070

Optimise SpecificationSet.getStatusCountsForProductSeries, fixing Product:+series timeouts.

postgres is inappropriately reluctant to use a BitmapOr with a subquery arm, so materialise the relatively small set of milestone IDs first.

-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/gSCFPS-optimise into lp:launchpad.

=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py	2015-09-30 01:51:52 +0000
+++ lib/lp/blueprints/interfaces/specification.py	2016-09-19 10:08:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Specification interfaces."""
@@ -749,7 +749,7 @@
         in the count.
 
         :param product_series: ProductSeries object.
-        :return: A list of tuples containing (status_id, count).
+        :return: A list of tuples containing (status, count).
         """
 
     def getByURL(url):

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2016-08-09 10:43:45 +0000
+++ lib/lp/blueprints/model/specification.py	2016-09-19 10:08:16 +0000
@@ -26,9 +26,11 @@
     SQLRelatedJoin,
     StringCol,
     )
-from storm.expr import Join
-from storm.locals import (
+from storm.expr import (
+    Count,
     Desc,
+    Join,
+    Or,
     SQL,
     )
 from storm.store import Store
@@ -87,6 +89,7 @@
 from lp.registry.interfaces.person import validate_public_person
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.productseries import IProductSeries
+from lp.registry.model.milestone import Milestone
 from lp.services.database import bulk
 from lp.services.database.constants import (
     DEFAULT,
@@ -97,7 +100,6 @@
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import (
     convert_storm_clause_to_string,
-    cursor,
     SQLBase,
     sqlvalues,
     )
@@ -1026,21 +1028,16 @@
 
     def getStatusCountsForProductSeries(self, product_series):
         """See `ISpecificationSet`."""
-        cur = cursor()
-        condition = """
-            (Specification.productseries = %s
-                 OR Milestone.productseries = %s)
-            """ % sqlvalues(product_series, product_series)
-        query = """
-            SELECT Specification.implementation_status, count(*)
-            FROM Specification
-                LEFT JOIN Milestone ON Specification.milestone = Milestone.id
-            WHERE
-                %s
-            GROUP BY Specification.implementation_status
-            """ % condition
-        cur.execute(query)
-        return cur.fetchall()
+        # Find specs targeted to the series or a milestone in the
+        # series. The milestone set is materialised client-side to
+        # get a good plan for the specification query.
+        return list(IStore(Specification).find(
+            (Specification.implementation_status, Count()),
+            Or(
+                Specification.productseries == product_series,
+                Specification.milestoneID.is_in(list(
+                    product_series.all_milestones.values(Milestone.id)))))
+            .group_by(Specification.implementation_status))
 
     def specifications(self, user, sort=None, quantity=None, filter=None,
                        need_people=True, need_branches=True,

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2015-09-29 00:05:21 +0000
+++ lib/lp/registry/browser/productseries.py	2016-09-19 10:08:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """View classes for `IProductSeries`."""
@@ -61,7 +61,6 @@
 from lp.blueprints.browser.specificationtarget import (
     HasSpecificationsMenuMixin,
     )
-from lp.blueprints.enums import SpecificationImplementationStatus
 from lp.blueprints.interfaces.specification import ISpecificationSet
 from lp.bugs.browser.bugtask import BugTargetTraversalMixin
 from lp.bugs.browser.structuralsubscription import (
@@ -448,11 +447,8 @@
     def specification_status_counts(self):
         """A list StatusCounts summarising the targeted specification."""
         specification_set = getUtility(ISpecificationSet)
-        status_id_counts = specification_set.getStatusCountsForProductSeries(
-            self.context)
-        SpecStatus = SpecificationImplementationStatus
-        status_counts = dict([(SpecStatus.items[status_id], count)
-                              for status_id, count in status_id_counts])
+        status_counts = dict(
+            specification_set.getStatusCountsForProductSeries(self.context))
         return [StatusCount(status, status_counts[status])
                 for status in sorted(status_counts,
                                      key=attrgetter('sortkey'))]

=== modified file 'lib/lp/registry/model/milestone.py'
--- lib/lp/registry/model/milestone.py	2015-07-08 16:05:11 +0000
+++ lib/lp/registry/model/milestone.py	2016-09-19 10:08:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Milestone model classes."""
@@ -38,12 +38,6 @@
 from zope.interface import implementer
 
 from lp.app.errors import NotFoundError
-from lp.blueprints.model.specification import Specification
-from lp.blueprints.model.specificationsearch import (
-    get_specification_active_product_filter,
-    get_specification_privacy_filter,
-    )
-from lp.blueprints.model.specificationworkitem import SpecificationWorkItem
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
 from lp.bugs.interfaces.bugtarget import IHasBugs
 from lp.bugs.interfaces.bugtask import (
@@ -161,11 +155,19 @@
 
     @property
     def all_specifications(self):
+        from lp.blueprints.model.specification import Specification
         return Store.of(self).find(
             Specification, Specification.milestoneID == self.id)
 
     def getSpecifications(self, user):
         """See `IMilestoneData`"""
+        from lp.blueprints.model.specification import Specification
+        from lp.blueprints.model.specificationsearch import (
+            get_specification_active_product_filter,
+            get_specification_privacy_filter,
+            )
+        from lp.blueprints.model.specificationworkitem import (
+            SpecificationWorkItem)
         from lp.registry.model.person import Person
         origin = [Specification]
         product_origin, clauses = get_specification_active_product_filter(