launchpad-reviewers team mailing list archive

[Colin Watson <mp+415667@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:empty-result-set-optimizations into launchpad:master.

Commit message:
Optimize away some trivially-empty queries

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/415667

This allows us to constrain some bug task query count tests (at least) more tightly.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:empty-result-set-optimizations into launchpad:master.

diff --git a/lib/lp/bugs/browser/tests/test_bugtask.py b/lib/lp/bugs/browser/tests/test_bugtask.py
index 55f3188..4a3b6fb 100644
--- a/lib/lp/bugs/browser/tests/test_bugtask.py
+++ b/lib/lp/bugs/browser/tests/test_bugtask.py
@@ -18,7 +18,7 @@ from testscenarios import (
     WithScenarios,
     )
 from testtools.matchers import (
-    LessThan,
+    Equals,
     Not,
     )
 import transaction
@@ -132,7 +132,7 @@ class TestBugTaskView(TestCaseWithFactory):
             0, 10, login_method=lambda: login(ADMIN_EMAIL))
         # This may seem large: it is; there is easily another 25% fat in
         # there.
-        self.assertThat(recorder1, HasQueryCount(LessThan(89)))
+        self.assertThat(recorder1, HasQueryCount(Equals(82)))
         self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
 
     def test_rendered_query_counts_constant_with_attachments(self):
@@ -143,7 +143,7 @@ class TestBugTaskView(TestCaseWithFactory):
             lambda: self.getUserBrowser(url, person),
             lambda: self.factory.makeBugAttachment(bug=task.bug),
             1, 9, login_method=lambda: login(ADMIN_EMAIL))
-        self.assertThat(recorder1, HasQueryCount(LessThan(90)))
+        self.assertThat(recorder1, HasQueryCount(Equals(83)))
         self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
 
     def makeLinkedBranchMergeProposal(self, sourcepackage, bug, owner):
@@ -178,11 +178,11 @@ class TestBugTaskView(TestCaseWithFactory):
         recorder1, recorder2 = record_two_runs(
             lambda: self.getUserBrowser(url, owner),
             make_merge_proposals, 0, 1)
-        self.assertThat(recorder1, HasQueryCount(LessThan(97)))
+        self.assertThat(recorder1, HasQueryCount(Equals(92)))
         # Ideally this should be much fewer, but this tries to keep a win of
         # removing more than half of these.
         self.assertThat(
-            recorder2, HasQueryCount(LessThan(recorder1.count + 41)))
+            recorder2, HasQueryCount(Equals(recorder1.count + 27)))
 
     def test_interesting_activity(self):
         # The interesting_activity property returns a tuple of interesting
@@ -224,7 +224,7 @@ class TestBugTaskView(TestCaseWithFactory):
             lambda: self.getUserBrowser(url, person),
             lambda: add_activity("description", self.factory.makePerson()),
             1, 20, login_method=lambda: login(ADMIN_EMAIL))
-        self.assertThat(recorder1, HasQueryCount(LessThan(90)))
+        self.assertThat(recorder1, HasQueryCount(Equals(83)))
         self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
 
     def test_rendered_query_counts_constant_with_milestones(self):
@@ -234,7 +234,7 @@ class TestBugTaskView(TestCaseWithFactory):
 
         with celebrity_logged_in('admin'):
             browses_under_limit = BrowsesWithQueryLimit(
-                90, self.factory.makePerson())
+                84, self.factory.makePerson())
 
         self.assertThat(bug, browses_under_limit)
 
@@ -2233,7 +2233,7 @@ class TestBugTaskSearchListingView(BrowserTestCase):
             lambda: self.getUserBrowser(url),
             lambda: self.factory.makeBug(target=product),
             1, 10, login_method=lambda: login(ANONYMOUS))
-        self.assertThat(recorder1, HasQueryCount(LessThan(47)))
+        self.assertThat(recorder1, HasQueryCount(Equals(45)))
         self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
 
     def test_mustache_model_in_json(self):
diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
index f3b08a6..ccbafd6 100644
--- a/lib/lp/bugs/model/bug.py
+++ b/lib/lp/bugs/model/bug.py
@@ -432,6 +432,8 @@ class Bug(SQLBase, InformationTypeMixin):
         xref_cve_sequences = [
             sequence for _, sequence in getUtility(IXRefSet).findFrom(
                 ('bug', str(self.id)), types=['cve'])]
+        if not xref_cve_sequences:
+            return []
         expr = Cve.sequence.is_in(xref_cve_sequences)
         return list(sorted(
             IStore(Cve).find(Cve, expr), key=operator.attrgetter('sequence')))
@@ -460,9 +462,12 @@ class Bug(SQLBase, InformationTypeMixin):
         from lp.blueprints.model.specificationsearch import (
             get_specification_privacy_filter,
             )
+        specifications = self.specifications
+        if not specifications:
+            return EmptyResultSet()
         return IStore(Specification).find(
             Specification,
-            Specification.id.is_in(spec.id for spec in self.specifications),
+            Specification.id.is_in(spec.id for spec in specifications),
             *get_specification_privacy_filter(user))
 
     @property
diff --git a/lib/lp/bugs/model/bugtask.py b/lib/lp/bugs/model/bugtask.py
index 7b31d6d..086299f 100644
--- a/lib/lp/bugs/model/bugtask.py
+++ b/lib/lp/bugs/model/bugtask.py
@@ -2005,13 +2005,9 @@ class BugTaskSet:
                 Milestone.productseriesID.is_in(product_series_ids)))
 
         # Pull in all the related pillars
-        list(store.find(
-            Distribution, Distribution.id.is_in(distro_ids)))
-        list(store.find(
-            DistroSeries, DistroSeries.id.is_in(distro_series_ids)))
-        list(store.find(
-            Product, Product.id.is_in(product_ids)))
-        list(store.find(
-            ProductSeries, ProductSeries.id.is_in(product_series_ids)))
+        load(Distribution, distro_ids)
+        load(DistroSeries, distro_series_ids)
+        load(Product, product_ids)
+        load(ProductSeries, product_series_ids)
 
         return milestones
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 1d5f2be..6a9eea7 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -1372,6 +1372,8 @@ class Person(
             team_id for team_id in team_ids
             if team_id not in self._inTeam_cache
         }
+        if not unknown_team_ids:
+            return False
 
         found_team_ids = set(
             IStore(TeamParticipation).find(