Merge lp:~cjwatson/launchpad/better-gitref-bulk-loading into lp:launchpad

Proposed by Colin Watson on 2018-08-31
Status: Merged
Merged at revision: 18777
Proposed branch: lp:~cjwatson/launchpad/better-gitref-bulk-loading
Merge into: lp:launchpad
Diff against target: 125 lines (+60/-8)
2 files modified
lib/lp/services/database/bulk.py (+33/-5)
lib/lp/services/database/tests/test_bulk.py (+27/-3)
To merge this branch: bzr merge lp:~cjwatson/launchpad/better-gitref-bulk-loading
Reviewer Review Type Date Requested Status
William Grant code 2018-08-31 Approve on 2018-09-12
Review via email: mp+354112@code.launchpad.net

Commit message

Use a more compact query to load objects with compound primary keys.

Description of the change

Mentioned in passing in bug 1790047, although it's not really essential to that bug.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)
18768. By Colin Watson on 2018-09-12

Document _make_compound_load_clause parameters.

Colin Watson (cjwatson) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/database/bulk.py'
2--- lib/lp/services/database/bulk.py 2013-08-23 05:36:46 +0000
3+++ lib/lp/services/database/bulk.py 2018-09-12 09:02:58 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2010 Canonical Ltd. This software is licensed under the
6+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """Optimized bulk operations against the database."""
10@@ -16,7 +16,10 @@
11
12 from collections import defaultdict
13 from functools import partial
14-from itertools import chain
15+from itertools import (
16+ chain,
17+ groupby,
18+ )
19 from operator import (
20 attrgetter,
21 itemgetter,
22@@ -99,6 +102,32 @@
23 return primary_key
24
25
26+def _make_compound_load_clause(primary_key, values_list):
27+ """Construct a bulk-loading query clause for a compound primary key.
28+
29+ When the primary key is compound, we expect that in practice we will
30+ often want to load objects with common leading elements of the key: for
31+ example, we often want to load many `GitRef` objects from the same
32+ repository. This helper returns a query clause optimised to be compact
33+ in this case.
34+
35+ :param primary_key: The primary key columns.
36+ :param values_list: A sequence of values of the primary key whose
37+ corresponding rows should be loaded. This must be sorted by the
38+ caller (because this function calls itself recursively, and it's
39+ usually more efficient to sort the whole sequence in one go).
40+ """
41+ if len(primary_key) > 1:
42+ return Or(*(
43+ And(
44+ primary_key[0] == leading_value,
45+ _make_compound_load_clause(
46+ primary_key[1:], [values[1:] for values in group]))
47+ for leading_value, group in groupby(values_list, itemgetter(0))))
48+ else:
49+ return primary_key[0].is_in([values[0] for values in values_list])
50+
51+
52 def load(object_type, primary_keys, store=None):
53 """Load a large number of objects efficiently."""
54 primary_key = _primary_key(object_type, allow_compound=True)
55@@ -107,9 +136,8 @@
56 if not primary_keys:
57 return []
58 if isinstance(primary_key, tuple):
59- condition = Or(*(
60- And(*(key == value for (key, value) in zip(primary_key, values)))
61- for values in primary_keys))
62+ condition = _make_compound_load_clause(
63+ primary_key, sorted(primary_keys))
64 else:
65 condition = primary_key.is_in(primary_keys)
66 if store is None:
67
68=== modified file 'lib/lp/services/database/tests/test_bulk.py'
69--- lib/lp/services/database/tests/test_bulk.py 2018-04-19 00:02:19 +0000
70+++ lib/lp/services/database/tests/test_bulk.py 2018-09-12 09:02:58 +0000
71@@ -1,4 +1,4 @@
72-# Copyright 2010 Canonical Ltd. This software is licensed under the
73+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
74 # GNU Affero General Public License version 3 (see the file LICENSE).
75
76 """Test the bulk database functions."""
77@@ -35,7 +35,10 @@
78 ISlaveStore,
79 IStore,
80 )
81-from lp.services.database.sqlbase import get_transaction_timestamp
82+from lp.services.database.sqlbase import (
83+ convert_storm_clause_to_string,
84+ get_transaction_timestamp,
85+ )
86 from lp.services.features.model import (
87 FeatureFlag,
88 getFeatureStore,
89@@ -172,6 +175,27 @@
90 bulk.reload([db_object])
91 self.assertIsNone(db_object_info.get('invalidated'))
92
93+ def test__make_compound_load_clause(self):
94+ # The query constructed by _make_compound_load_clause has the
95+ # correct structure.
96+ clause = bulk._make_compound_load_clause(
97+ # This primary key is fictional, but lets us do a more elaborate
98+ # test.
99+ (FeatureFlag.scope, FeatureFlag.priority, FeatureFlag.flag),
100+ sorted(
101+ [(u'foo', 0, u'bar'), (u'foo', 0, u'baz'),
102+ (u'foo', 1, u'bar'), (u'foo', 1, u'quux'),
103+ (u'bar', 0, u'foo')]))
104+ self.assertEqual(
105+ "FeatureFlag.scope = E'bar' AND ("
106+ "FeatureFlag.priority = 0 AND FeatureFlag.flag IN (E'foo')) OR "
107+ "FeatureFlag.scope = E'foo' AND ("
108+ "FeatureFlag.priority = 0 AND "
109+ "FeatureFlag.flag IN (E'bar', E'baz') OR "
110+ "FeatureFlag.priority = 1 AND "
111+ "FeatureFlag.flag IN (E'bar', E'quux'))",
112+ convert_storm_clause_to_string(clause))
113+
114 def test_load(self):
115 # load() loads objects of the given type by their primary keys.
116 db_objects = [
117@@ -189,7 +213,7 @@
118 ClassInfoError, bulk.load, str, [])
119
120 def test_load_with_compound_primary_keys(self):
121- # load() does not like compound primary keys.
122+ # load() can load objects with compound primary keys.
123 flags = [
124 FeatureFlag(u'foo', 0, u'bar', u'true'),
125 FeatureFlag(u'foo', 0, u'baz', u'false'),