Launchpad itself

Merge ~cjwatson/launchpad:empty-result-set-optimizations into launchpad:master

  1. Git
  2. lp:~cjwatson/launchpad
  3. empty-result-set-optimizations
  4. Merge into master
Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: e1d3388ddc4212b89406c7a218c3b0865cf64d49
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:empty-result-set-optimizations
Merge into: launchpad:master
Diff against target: 134 lines (+20/-17)
4 files modified
lib/lp/bugs/browser/tests/test_bugtask.py (+8/-8)
lib/lp/bugs/model/bug.py (+6/-1)
lib/lp/bugs/model/bugtask.py (+4/-8)
lib/lp/registry/model/person.py (+2/-0)
Reviewer Review Type Date Requested Status
Guruprasad Approve
Review via email: mp+415667@code.launchpad.net

Commit message

Optimize away some trivially-empty queries

Description of the change

This allows us to constrain some bug task query count tests (at least) more tightly.

To post a comment you must log in.
Revision history for this message
Guruprasad (lgp171188) wrote :

LGTM 👍

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/bugs/browser/tests/test_bugtask.py b/lib/lp/bugs/browser/tests/test_bugtask.py
2index 55f3188..4a3b6fb 100644
3--- a/lib/lp/bugs/browser/tests/test_bugtask.py
4+++ b/lib/lp/bugs/browser/tests/test_bugtask.py
5@@ -18,7 +18,7 @@ from testscenarios import (
6 WithScenarios,
7 )
8 from testtools.matchers import (
9- LessThan,
10+ Equals,
11 Not,
12 )
13 import transaction
14@@ -132,7 +132,7 @@ class TestBugTaskView(TestCaseWithFactory):
15 0, 10, login_method=lambda: login(ADMIN_EMAIL))
16 # This may seem large: it is; there is easily another 25% fat in
17 # there.
18- self.assertThat(recorder1, HasQueryCount(LessThan(89)))
19+ self.assertThat(recorder1, HasQueryCount(Equals(82)))
20 self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
21
22 def test_rendered_query_counts_constant_with_attachments(self):
23@@ -143,7 +143,7 @@ class TestBugTaskView(TestCaseWithFactory):
24 lambda: self.getUserBrowser(url, person),
25 lambda: self.factory.makeBugAttachment(bug=task.bug),
26 1, 9, login_method=lambda: login(ADMIN_EMAIL))
27- self.assertThat(recorder1, HasQueryCount(LessThan(90)))
28+ self.assertThat(recorder1, HasQueryCount(Equals(83)))
29 self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
30
31 def makeLinkedBranchMergeProposal(self, sourcepackage, bug, owner):
32@@ -178,11 +178,11 @@ class TestBugTaskView(TestCaseWithFactory):
33 recorder1, recorder2 = record_two_runs(
34 lambda: self.getUserBrowser(url, owner),
35 make_merge_proposals, 0, 1)
36- self.assertThat(recorder1, HasQueryCount(LessThan(97)))
37+ self.assertThat(recorder1, HasQueryCount(Equals(92)))
38 # Ideally this should be much fewer, but this tries to keep a win of
39 # removing more than half of these.
40 self.assertThat(
41- recorder2, HasQueryCount(LessThan(recorder1.count + 41)))
42+ recorder2, HasQueryCount(Equals(recorder1.count + 27)))
43
44 def test_interesting_activity(self):
45 # The interesting_activity property returns a tuple of interesting
46@@ -224,7 +224,7 @@ class TestBugTaskView(TestCaseWithFactory):
47 lambda: self.getUserBrowser(url, person),
48 lambda: add_activity("description", self.factory.makePerson()),
49 1, 20, login_method=lambda: login(ADMIN_EMAIL))
50- self.assertThat(recorder1, HasQueryCount(LessThan(90)))
51+ self.assertThat(recorder1, HasQueryCount(Equals(83)))
52 self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
53
54 def test_rendered_query_counts_constant_with_milestones(self):
55@@ -234,7 +234,7 @@ class TestBugTaskView(TestCaseWithFactory):
56
57 with celebrity_logged_in('admin'):
58 browses_under_limit = BrowsesWithQueryLimit(
59- 90, self.factory.makePerson())
60+ 84, self.factory.makePerson())
61
62 self.assertThat(bug, browses_under_limit)
63
64@@ -2233,7 +2233,7 @@ class TestBugTaskSearchListingView(BrowserTestCase):
65 lambda: self.getUserBrowser(url),
66 lambda: self.factory.makeBug(target=product),
67 1, 10, login_method=lambda: login(ANONYMOUS))
68- self.assertThat(recorder1, HasQueryCount(LessThan(47)))
69+ self.assertThat(recorder1, HasQueryCount(Equals(45)))
70 self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
71
72 def test_mustache_model_in_json(self):
73diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
74index f3b08a6..ccbafd6 100644
75--- a/lib/lp/bugs/model/bug.py
76+++ b/lib/lp/bugs/model/bug.py
77@@ -432,6 +432,8 @@ class Bug(SQLBase, InformationTypeMixin):
78 xref_cve_sequences = [
79 sequence for _, sequence in getUtility(IXRefSet).findFrom(
80 ('bug', str(self.id)), types=['cve'])]
81+ if not xref_cve_sequences:
82+ return []
83 expr = Cve.sequence.is_in(xref_cve_sequences)
84 return list(sorted(
85 IStore(Cve).find(Cve, expr), key=operator.attrgetter('sequence')))
86@@ -460,9 +462,12 @@ class Bug(SQLBase, InformationTypeMixin):
87 from lp.blueprints.model.specificationsearch import (
88 get_specification_privacy_filter,
89 )
90+ specifications = self.specifications
91+ if not specifications:
92+ return EmptyResultSet()
93 return IStore(Specification).find(
94 Specification,
95- Specification.id.is_in(spec.id for spec in self.specifications),
96+ Specification.id.is_in(spec.id for spec in specifications),
97 *get_specification_privacy_filter(user))
98
99 @property
100diff --git a/lib/lp/bugs/model/bugtask.py b/lib/lp/bugs/model/bugtask.py
101index 7b31d6d..086299f 100644
102--- a/lib/lp/bugs/model/bugtask.py
103+++ b/lib/lp/bugs/model/bugtask.py
104@@ -2005,13 +2005,9 @@ class BugTaskSet:
105 Milestone.productseriesID.is_in(product_series_ids)))
106
107 # Pull in all the related pillars
108- list(store.find(
109- Distribution, Distribution.id.is_in(distro_ids)))
110- list(store.find(
111- DistroSeries, DistroSeries.id.is_in(distro_series_ids)))
112- list(store.find(
113- Product, Product.id.is_in(product_ids)))
114- list(store.find(
115- ProductSeries, ProductSeries.id.is_in(product_series_ids)))
116+ load(Distribution, distro_ids)
117+ load(DistroSeries, distro_series_ids)
118+ load(Product, product_ids)
119+ load(ProductSeries, product_series_ids)
120
121 return milestones
122diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
123index 1d5f2be..6a9eea7 100644
124--- a/lib/lp/registry/model/person.py
125+++ b/lib/lp/registry/model/person.py
126@@ -1372,6 +1372,8 @@ class Person(
127 team_id for team_id in team_ids
128 if team_id not in self._inTeam_cache
129 }
130+ if not unknown_team_ids:
131+ return False
132
133 found_team_ids = set(
134 IStore(TeamParticipation).find(

Subscribers

People subscribed via source and target branches

to status/vote changes: