Merge lp:~edoardo-serra/storm/select-for-update-support into lp:storm
- select-for-update-support
- Merge into trunk
Status: | Needs review |
---|---|
Proposed branch: | lp:~edoardo-serra/storm/select-for-update-support |
Merge into: | lp:storm |
Diff against target: |
510 lines (+188/-18) 6 files modified
storm/databases/sqlite.py (+4/-0) storm/expr.py (+7/-3) storm/store.py (+51/-14) storm/zope/interfaces.py (+3/-0) tests/expr.py (+13/-1) tests/store/base.py (+110/-0) |
To merge this branch: | bzr merge lp:~edoardo-serra/storm/select-for-update-support |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Storm Developers | Pending | ||
Storm Developers | Pending | ||
Review via email: mp+35112@code.launchpad.net |
Commit message
Description of the change
This branches implements the SELECT ... FOR UPDATE
Edoardo Serra (edoardo-serra) wrote : | # |
Hi Jamu, sorry I didn't find any mention of this on the Wiki.
I just accepdet the form via email.
Regards
On Sep 10, 2010, at 4:52 PM, Jamu Kakar wrote:
> Edoardo, thanks for this branch! Before we spend time reviewing the
> changes, have you signed the Canonical copyright assignment forms?
> This process will need to be completed before we can accept changes.
>
> --
> https:/
> You are the owner of lp:~edoardo-serra/storm/select-for-update-support.
James Henstridge (jamesh) wrote : | # |
This isn't a full review, but here are a few initial comments:
1. If "SELECT FOR UPDATE" is incompatible with "GROUP BY", perhaps it would make sense to check for that in Select's compile function and raise ExprError there rather than trying to protect all the ResultSet entry points that could cause this problem.
2. You've added a comment asking whether it makes sense to remove the "FOR UPDATE" clause for ResultSet.
3. This doesn't need to go in your branch, but I wonder if it would be worth storing the face that a row has been locked in its obj_info, so that Store.get(..., for_update=True) can avoid a query if the row has already been locked? This could be set in Store._
James Henstridge (jamesh) wrote : | # |
One other point: it is probably worth using an API that can support "FOR SHARE" read locks in addition to "FOR UPDATE" write locks.
Unmerged revisions
- 380. By Edoardo Serra <eserra@barbera>
-
SELECT ... FOR UPDATE implemented in Store.get
- 379. By Edoardo Serra <eserra@barbera>
-
Unit tests
- 378. By Edoardo Serra <eserra@barbera>
-
SELECT ... FOR UPDATE is not compatible with GROUP BY
- 377. By Edoardo Serra <eserra@barbera>
-
SQLite does not support SELECT ... FOR UPDATE
- 376. By Edoardo Serra <eserra@barbera>
-
Starting implementation of SELECT ... FOR UPATE
Preview Diff
1 | === modified file 'storm/databases/sqlite.py' | |||
2 | --- storm/databases/sqlite.py 2010-06-14 11:52:25 +0000 | |||
3 | +++ storm/databases/sqlite.py 2010-09-10 14:44:22 +0000 | |||
4 | @@ -49,6 +49,10 @@ | |||
5 | 49 | def compile_select_sqlite(compile, select, state): | 49 | def compile_select_sqlite(compile, select, state): |
6 | 50 | if select.offset is not Undef and select.limit is Undef: | 50 | if select.offset is not Undef and select.limit is Undef: |
7 | 51 | select.limit = sys.maxint | 51 | select.limit = sys.maxint |
8 | 52 | # SQLite does not support SELECT ... FOR UPDATE | ||
9 | 53 | # Can we just ignore it? | ||
10 | 54 | if select.for_update: | ||
11 | 55 | select.for_update = False | ||
12 | 52 | statement = compile_select(compile, select, state) | 56 | statement = compile_select(compile, select, state) |
13 | 53 | if state.context is SELECT: | 57 | if state.context is SELECT: |
14 | 54 | # SQLite breaks with (SELECT ...) UNION (SELECT ...), so we | 58 | # SQLite breaks with (SELECT ...) UNION (SELECT ...), so we |
15 | 55 | 59 | ||
16 | === modified file 'storm/expr.py' | |||
17 | --- storm/expr.py 2009-11-02 11:11:20 +0000 | |||
18 | +++ storm/expr.py 2010-09-10 14:44:22 +0000 | |||
19 | @@ -636,12 +636,13 @@ | |||
20 | 636 | 636 | ||
21 | 637 | class Select(Expr): | 637 | class Select(Expr): |
22 | 638 | __slots__ = ("columns", "where", "tables", "default_tables", "order_by", | 638 | __slots__ = ("columns", "where", "tables", "default_tables", "order_by", |
24 | 639 | "group_by", "limit", "offset", "distinct", "having") | 639 | "group_by", "limit", "offset", "distinct", "having", |
25 | 640 | 'for_update') | ||
26 | 640 | 641 | ||
27 | 641 | def __init__(self, columns, where=Undef, | 642 | def __init__(self, columns, where=Undef, |
28 | 642 | tables=Undef, default_tables=Undef, | 643 | tables=Undef, default_tables=Undef, |
31 | 643 | order_by=Undef, group_by=Undef, | 644 | order_by=Undef, group_by=Undef, limit=Undef, |
32 | 644 | limit=Undef, offset=Undef, distinct=False, having=Undef): | 645 | offset=Undef, distinct=False, having=Undef, for_update=False): |
33 | 645 | self.columns = columns | 646 | self.columns = columns |
34 | 646 | self.where = where | 647 | self.where = where |
35 | 647 | self.tables = tables | 648 | self.tables = tables |
36 | @@ -652,6 +653,7 @@ | |||
37 | 652 | self.offset = offset | 653 | self.offset = offset |
38 | 653 | self.distinct = distinct | 654 | self.distinct = distinct |
39 | 654 | self.having = having | 655 | self.having = having |
40 | 656 | self.for_update = for_update | ||
41 | 655 | 657 | ||
42 | 656 | @compile.when(Select) | 658 | @compile.when(Select) |
43 | 657 | def compile_select(compile, select, state): | 659 | def compile_select(compile, select, state): |
44 | @@ -680,6 +682,8 @@ | |||
45 | 680 | tokens.append(" LIMIT %d" % select.limit) | 682 | tokens.append(" LIMIT %d" % select.limit) |
46 | 681 | if select.offset is not Undef: | 683 | if select.offset is not Undef: |
47 | 682 | tokens.append(" OFFSET %d" % select.offset) | 684 | tokens.append(" OFFSET %d" % select.offset) |
48 | 685 | if select.for_update: | ||
49 | 686 | tokens.append(" FOR UPDATE") | ||
50 | 683 | if has_tables(state, select): | 687 | if has_tables(state, select): |
51 | 684 | state.context = TABLE | 688 | state.context = TABLE |
52 | 685 | state.push("parameters", []) | 689 | state.push("parameters", []) |
53 | 686 | 690 | ||
54 | === modified file 'storm/store.py' | |||
55 | --- storm/store.py 2010-09-01 15:00:42 +0000 | |||
56 | +++ storm/store.py 2010-09-10 14:44:22 +0000 | |||
57 | @@ -137,13 +137,16 @@ | |||
58 | 137 | self.invalidate() | 137 | self.invalidate() |
59 | 138 | self._connection.rollback() | 138 | self._connection.rollback() |
60 | 139 | 139 | ||
62 | 140 | def get(self, cls, key): | 140 | def get(self, cls, key, for_update=False): |
63 | 141 | """Get object of type cls with the given primary key from the database. | 141 | """Get object of type cls with the given primary key from the database. |
64 | 142 | 142 | ||
65 | 143 | If the object is alive the database won't be touched. | 143 | If the object is alive the database won't be touched. |
66 | 144 | 144 | ||
67 | 145 | If the FOR UPDATE clause is used, the query is always executed | ||
68 | 146 | |||
69 | 145 | @param cls: Class of the object to be retrieved. | 147 | @param cls: Class of the object to be retrieved. |
70 | 146 | @param key: Primary key of object. May be a tuple for composed keys. | 148 | @param key: Primary key of object. May be a tuple for composed keys. |
71 | 149 | @param for_update: Indicates wether using the clause FOR UPDATE | ||
72 | 147 | 150 | ||
73 | 148 | @return: The object found with the given primary key, or None | 151 | @return: The object found with the given primary key, or None |
74 | 149 | if no object is found. | 152 | if no object is found. |
75 | @@ -165,20 +168,22 @@ | |||
76 | 165 | variable = column.variable_factory(value=variable) | 168 | variable = column.variable_factory(value=variable) |
77 | 166 | primary_vars.append(variable) | 169 | primary_vars.append(variable) |
78 | 167 | 170 | ||
88 | 168 | primary_values = tuple(var.get(to_db=True) for var in primary_vars) | 171 | if not for_update: |
89 | 169 | obj_info = self._alive.get((cls_info.cls, primary_values)) | 172 | primary_values = tuple(var.get(to_db=True) for var in primary_vars) |
90 | 170 | if obj_info is not None: | 173 | obj_info = self._alive.get((cls_info.cls, primary_values)) |
91 | 171 | if obj_info.get("invalidated"): | 174 | if obj_info is not None: |
92 | 172 | try: | 175 | if obj_info.get("invalidated"): |
93 | 173 | self._validate_alive(obj_info) | 176 | try: |
94 | 174 | except LostObjectError: | 177 | self._validate_alive(obj_info) |
95 | 175 | return None | 178 | except LostObjectError: |
96 | 176 | return self._get_object(obj_info) | 179 | return None |
97 | 180 | return self._get_object(obj_info) | ||
98 | 177 | 181 | ||
99 | 178 | where = compare_columns(cls_info.primary_key, primary_vars) | 182 | where = compare_columns(cls_info.primary_key, primary_vars) |
100 | 179 | 183 | ||
101 | 180 | select = Select(cls_info.columns, where, | 184 | select = Select(cls_info.columns, where, |
103 | 181 | default_tables=cls_info.table, limit=1) | 185 | default_tables=cls_info.table, limit=1, |
104 | 186 | for_update=for_update) | ||
105 | 182 | 187 | ||
106 | 183 | result = self._connection.execute(select) | 188 | result = self._connection.execute(select) |
107 | 184 | values = result.get_one() | 189 | values = result.get_one() |
108 | @@ -921,6 +926,7 @@ | |||
109 | 921 | self._distinct = False | 926 | self._distinct = False |
110 | 922 | self._group_by = Undef | 927 | self._group_by = Undef |
111 | 923 | self._having = Undef | 928 | self._having = Undef |
112 | 929 | self._for_update = False | ||
113 | 924 | 930 | ||
114 | 925 | def copy(self): | 931 | def copy(self): |
115 | 926 | """Return a copy of this ResultSet object, with the same configuration. | 932 | """Return a copy of this ResultSet object, with the same configuration. |
116 | @@ -933,7 +939,7 @@ | |||
117 | 933 | result_set._select = copy(self._select) | 939 | result_set._select = copy(self._select) |
118 | 934 | return result_set | 940 | return result_set |
119 | 935 | 941 | ||
121 | 936 | def config(self, distinct=None, offset=None, limit=None): | 942 | def config(self, distinct=None, offset=None, limit=None, for_update=None): |
122 | 937 | """Configure this result object in-place. All parameters are optional. | 943 | """Configure this result object in-place. All parameters are optional. |
123 | 938 | 944 | ||
124 | 939 | @param distinct: Boolean enabling/disabling usage of the DISTINCT | 945 | @param distinct: Boolean enabling/disabling usage of the DISTINCT |
125 | @@ -952,6 +958,8 @@ | |||
126 | 952 | self._offset = offset | 958 | self._offset = offset |
127 | 953 | if limit is not None: | 959 | if limit is not None: |
128 | 954 | self._limit = limit | 960 | self._limit = limit |
129 | 961 | if for_update is not None: | ||
130 | 962 | self._for_update = for_update | ||
131 | 955 | return self | 963 | return self |
132 | 956 | 964 | ||
133 | 957 | def _get_select(self): | 965 | def _get_select(self): |
134 | @@ -967,7 +975,7 @@ | |||
135 | 967 | return Select(columns, self._where, self._tables, default_tables, | 975 | return Select(columns, self._where, self._tables, default_tables, |
136 | 968 | self._order_by, offset=self._offset, limit=self._limit, | 976 | self._order_by, offset=self._offset, limit=self._limit, |
137 | 969 | distinct=self._distinct, group_by=self._group_by, | 977 | distinct=self._distinct, group_by=self._group_by, |
139 | 970 | having=self._having) | 978 | having=self._having, for_update=self._for_update) |
140 | 971 | 979 | ||
141 | 972 | def _load_objects(self, result, values): | 980 | def _load_objects(self, result, values): |
142 | 973 | return self._find_spec.load_objects(self._store, result, values) | 981 | return self._find_spec.load_objects(self._store, result, values) |
143 | @@ -1054,6 +1062,9 @@ | |||
144 | 1054 | subselect = self._get_select() | 1062 | subselect = self._get_select() |
145 | 1055 | subselect.limit = 1 | 1063 | subselect.limit = 1 |
146 | 1056 | subselect.order_by = Undef | 1064 | subselect.order_by = Undef |
147 | 1065 | # Should we really strip FOR UPDATE clause or should we raise a | ||
148 | 1066 | # FeatureError? | ||
149 | 1067 | subselect.for_update = False | ||
150 | 1057 | select = Select(1, tables=Alias(subselect, "_tmp"), limit=1) | 1068 | select = Select(1, tables=Alias(subselect, "_tmp"), limit=1) |
151 | 1058 | result = self._store._connection.execute(select) | 1069 | result = self._store._connection.execute(select) |
152 | 1059 | return (not result.get_one()) | 1070 | return (not result.get_one()) |
153 | @@ -1162,6 +1173,15 @@ | |||
154 | 1162 | self._order_by = args or Undef | 1173 | self._order_by = args or Undef |
155 | 1163 | return self | 1174 | return self |
156 | 1164 | 1175 | ||
157 | 1176 | def for_update(self): | ||
158 | 1177 | """Acquire an exclusive row-level lock on the rows in the ResultSet | ||
159 | 1178 | """ | ||
160 | 1179 | if self._group_by is not Undef: | ||
161 | 1180 | raise FeatureError("SELECT ... FOR UPDATE isn't supported with a " | ||
162 | 1181 | " GROUP BY clause ") | ||
163 | 1182 | self._for_update = True | ||
164 | 1183 | return self | ||
165 | 1184 | |||
166 | 1165 | def remove(self): | 1185 | def remove(self): |
167 | 1166 | """Remove all rows represented by this ResultSet from the database. | 1186 | """Remove all rows represented by this ResultSet from the database. |
168 | 1167 | 1187 | ||
169 | @@ -1193,6 +1213,9 @@ | |||
170 | 1193 | if self._select is not Undef: | 1213 | if self._select is not Undef: |
171 | 1194 | raise FeatureError("Grouping isn't supported with " | 1214 | raise FeatureError("Grouping isn't supported with " |
172 | 1195 | "set expressions (unions, etc)") | 1215 | "set expressions (unions, etc)") |
173 | 1216 | if self._for_update: | ||
174 | 1217 | raise FeatureError("Grouping isn't supported with " | ||
175 | 1218 | "FOR UPDATE clause") | ||
176 | 1196 | 1219 | ||
177 | 1197 | find_spec = FindSpec(expr) | 1220 | find_spec = FindSpec(expr) |
178 | 1198 | columns, dummy = find_spec.get_columns_and_tables() | 1221 | columns, dummy = find_spec.get_columns_and_tables() |
179 | @@ -1216,6 +1239,9 @@ | |||
180 | 1216 | if self._group_by is not Undef: | 1239 | if self._group_by is not Undef: |
181 | 1217 | raise FeatureError("Single aggregates aren't supported after a " | 1240 | raise FeatureError("Single aggregates aren't supported after a " |
182 | 1218 | " GROUP BY clause ") | 1241 | " GROUP BY clause ") |
183 | 1242 | if self._for_update: | ||
184 | 1243 | raise FeatureError("Single aggregates aren't supported with" | ||
185 | 1244 | " FOR UPDATE clause ") | ||
186 | 1219 | columns, default_tables = self._find_spec.get_columns_and_tables() | 1245 | columns, default_tables = self._find_spec.get_columns_and_tables() |
187 | 1220 | if (self._select is Undef and not self._distinct and | 1246 | if (self._select is Undef and not self._distinct and |
188 | 1221 | self._offset is Undef and self._limit is Undef): | 1247 | self._offset is Undef and self._limit is Undef): |
189 | @@ -1471,6 +1497,8 @@ | |||
190 | 1471 | """ | 1497 | """ |
191 | 1472 | if isinstance(other, EmptyResultSet): | 1498 | if isinstance(other, EmptyResultSet): |
192 | 1473 | return self | 1499 | return self |
193 | 1500 | if self._for_update or other._for_update: | ||
194 | 1501 | raise FeatureError("SELECT FOR UPDATE is not allowed with UNION") | ||
195 | 1474 | return self._set_expr(Union, other, all) | 1502 | return self._set_expr(Union, other, all) |
196 | 1475 | 1503 | ||
197 | 1476 | def difference(self, other, all=False): | 1504 | def difference(self, other, all=False): |
198 | @@ -1480,6 +1508,8 @@ | |||
199 | 1480 | """ | 1508 | """ |
200 | 1481 | if isinstance(other, EmptyResultSet): | 1509 | if isinstance(other, EmptyResultSet): |
201 | 1482 | return self | 1510 | return self |
202 | 1511 | if self._for_update or other._for_update: | ||
203 | 1512 | raise FeatureError("SELECT FOR UPDATE is not allowed with EXCEPT") | ||
204 | 1483 | return self._set_expr(Except, other, all) | 1513 | return self._set_expr(Except, other, all) |
205 | 1484 | 1514 | ||
206 | 1485 | def intersection(self, other, all=False): | 1515 | def intersection(self, other, all=False): |
207 | @@ -1489,6 +1519,9 @@ | |||
208 | 1489 | """ | 1519 | """ |
209 | 1490 | if isinstance(other, EmptyResultSet): | 1520 | if isinstance(other, EmptyResultSet): |
210 | 1491 | return other | 1521 | return other |
211 | 1522 | if self._for_update or other._for_update: | ||
212 | 1523 | raise FeatureError("SELECT FOR UPDATE is not allowed with " | ||
213 | 1524 | "INTERSECT") | ||
214 | 1492 | return self._set_expr(Intersect, other, all) | 1525 | return self._set_expr(Intersect, other, all) |
215 | 1493 | 1526 | ||
216 | 1494 | 1527 | ||
217 | @@ -1516,7 +1549,7 @@ | |||
218 | 1516 | result = EmptyResultSet(self._order_by) | 1549 | result = EmptyResultSet(self._order_by) |
219 | 1517 | return result | 1550 | return result |
220 | 1518 | 1551 | ||
222 | 1519 | def config(self, distinct=None, offset=None, limit=None): | 1552 | def config(self, distinct=None, offset=None, limit=None, for_update=None): |
223 | 1520 | pass | 1553 | pass |
224 | 1521 | 1554 | ||
225 | 1522 | def __iter__(self): | 1555 | def __iter__(self): |
226 | @@ -1552,6 +1585,10 @@ | |||
227 | 1552 | self._order_by = True | 1585 | self._order_by = True |
228 | 1553 | return self | 1586 | return self |
229 | 1554 | 1587 | ||
230 | 1588 | def for_update(self): | ||
231 | 1589 | self._for_update = True | ||
232 | 1590 | return self | ||
233 | 1591 | |||
234 | 1555 | def remove(self): | 1592 | def remove(self): |
235 | 1556 | return 0 | 1593 | return 0 |
236 | 1557 | 1594 | ||
237 | 1558 | 1595 | ||
238 | === modified file 'storm/zope/interfaces.py' | |||
239 | --- storm/zope/interfaces.py 2008-11-27 09:37:56 +0000 | |||
240 | +++ storm/zope/interfaces.py 2010-09-10 14:44:22 +0000 | |||
241 | @@ -93,6 +93,9 @@ | |||
242 | 93 | def order_by(*args): | 93 | def order_by(*args): |
243 | 94 | """Order the result set based on expressions in C{args}.""" | 94 | """Order the result set based on expressions in C{args}.""" |
244 | 95 | 95 | ||
245 | 96 | def for_update(): | ||
246 | 97 | """Acquire an exclusive row-level lock on the rows in the ResultSet""" | ||
247 | 98 | |||
248 | 96 | def count(column=Undef, distinct=False): | 99 | def count(column=Undef, distinct=False): |
249 | 97 | """Returns the number of rows in the result set. | 100 | """Returns the number of rows in the result set. |
250 | 98 | 101 | ||
251 | 99 | 102 | ||
252 | === modified file 'tests/expr.py' | |||
253 | --- tests/expr.py 2009-11-02 11:11:20 +0000 | |||
254 | +++ tests/expr.py 2010-09-10 14:44:22 +0000 | |||
255 | @@ -65,9 +65,11 @@ | |||
256 | 65 | self.assertEquals(expr.limit, Undef) | 65 | self.assertEquals(expr.limit, Undef) |
257 | 66 | self.assertEquals(expr.offset, Undef) | 66 | self.assertEquals(expr.offset, Undef) |
258 | 67 | self.assertEquals(expr.distinct, False) | 67 | self.assertEquals(expr.distinct, False) |
259 | 68 | self.assertEquals(expr.having, Undef) | ||
260 | 69 | self.assertEquals(expr.for_update, False) | ||
261 | 68 | 70 | ||
262 | 69 | def test_select_constructor(self): | 71 | def test_select_constructor(self): |
264 | 70 | objects = [object() for i in range(9)] | 72 | objects = [object() for i in range(11)] |
265 | 71 | expr = Select(*objects) | 73 | expr = Select(*objects) |
266 | 72 | self.assertEquals(expr.columns, objects[0]) | 74 | self.assertEquals(expr.columns, objects[0]) |
267 | 73 | self.assertEquals(expr.where, objects[1]) | 75 | self.assertEquals(expr.where, objects[1]) |
268 | @@ -78,6 +80,8 @@ | |||
269 | 78 | self.assertEquals(expr.limit, objects[6]) | 80 | self.assertEquals(expr.limit, objects[6]) |
270 | 79 | self.assertEquals(expr.offset, objects[7]) | 81 | self.assertEquals(expr.offset, objects[7]) |
271 | 80 | self.assertEquals(expr.distinct, objects[8]) | 82 | self.assertEquals(expr.distinct, objects[8]) |
272 | 83 | self.assertEquals(expr.having, objects[9]) | ||
273 | 84 | self.assertEquals(expr.for_update, objects[10]) | ||
274 | 81 | 85 | ||
275 | 82 | def test_insert_default(self): | 86 | def test_insert_default(self): |
276 | 83 | expr = Insert(None) | 87 | expr = Insert(None) |
277 | @@ -657,6 +661,14 @@ | |||
278 | 657 | 'SELECT DISTINCT column1, column2 FROM "table 1"') | 661 | 'SELECT DISTINCT column1, column2 FROM "table 1"') |
279 | 658 | self.assertEquals(state.parameters, []) | 662 | self.assertEquals(state.parameters, []) |
280 | 659 | 663 | ||
281 | 664 | def test_select_for_update(self): | ||
282 | 665 | expr = Select([column1, column2], Undef, [table1], for_update=True) | ||
283 | 666 | state = State() | ||
284 | 667 | statement = compile(expr, state) | ||
285 | 668 | self.assertEquals(statement, | ||
286 | 669 | 'SELECT column1, column2 FROM "table 1" FOR UPDATE') | ||
287 | 670 | self.assertEquals(state.parameters, []) | ||
288 | 671 | |||
289 | 660 | def test_select_where(self): | 672 | def test_select_where(self): |
290 | 661 | expr = Select([column1, Func1()], | 673 | expr = Select([column1, Func1()], |
291 | 662 | Func1(), | 674 | Func1(), |
292 | 663 | 675 | ||
293 | === modified file 'tests/store/base.py' | |||
294 | --- tests/store/base.py 2010-09-01 09:02:41 +0000 | |||
295 | +++ tests/store/base.py 2010-09-10 14:44:22 +0000 | |||
296 | @@ -321,6 +321,18 @@ | |||
297 | 321 | foo = self.store.get(Foo, 40) | 321 | foo = self.store.get(Foo, 40) |
298 | 322 | self.assertEquals(foo, None) | 322 | self.assertEquals(foo, None) |
299 | 323 | 323 | ||
300 | 324 | def test_get_for_update(self): | ||
301 | 325 | stream = StringIO() | ||
302 | 326 | self.addCleanup(debug, False) | ||
303 | 327 | debug(True, stream) | ||
304 | 328 | |||
305 | 329 | foo = self.store.get(Foo, 10, for_update=True) | ||
306 | 330 | self.assertEquals(foo.id, 10) | ||
307 | 331 | self.assertEquals(foo.title, "Title 30") | ||
308 | 332 | if self.__class__.__name__.startswith("SQLite"): | ||
309 | 333 | return | ||
310 | 334 | self.assertIn("FOR UPDATE", stream.getvalue()) | ||
311 | 335 | |||
312 | 324 | def test_get_cached(self): | 336 | def test_get_cached(self): |
313 | 325 | foo = self.store.get(Foo, 10) | 337 | foo = self.store.get(Foo, 10) |
314 | 326 | self.assertTrue(self.store.get(Foo, 10) is foo) | 338 | self.assertTrue(self.store.get(Foo, 10) is foo) |
315 | @@ -332,6 +344,13 @@ | |||
316 | 332 | self.store.get(Foo, 10) | 344 | self.store.get(Foo, 10) |
317 | 333 | self.store._connection = connection | 345 | self.store._connection = connection |
318 | 334 | 346 | ||
319 | 347 | def test_wb_get_cached_for_update_needa_connection(self): | ||
320 | 348 | foo = self.store.get(Foo, 10) | ||
321 | 349 | connection = self.store._connection | ||
322 | 350 | self.store._connection = None | ||
323 | 351 | self.assertRaises(AttributeError, self.store.get, Foo, 10, for_update=True) | ||
324 | 352 | self.store._connection = connection | ||
325 | 353 | |||
326 | 335 | def test_cache_cleanup(self): | 354 | def test_cache_cleanup(self): |
327 | 336 | # Disable the cache, which holds strong references. | 355 | # Disable the cache, which holds strong references. |
328 | 337 | self.get_cache(self.store).set_size(0) | 356 | self.get_cache(self.store).set_size(0) |
329 | @@ -569,6 +588,21 @@ | |||
330 | 569 | self.assertEqual(True, result.is_empty()) | 588 | self.assertEqual(True, result.is_empty()) |
331 | 570 | self.assertNotIn("ORDER BY", stream.getvalue()) | 589 | self.assertNotIn("ORDER BY", stream.getvalue()) |
332 | 571 | 590 | ||
333 | 591 | def test_is_empty_strips_for_update(self): | ||
334 | 592 | """ | ||
335 | 593 | L{ResultSet.is_empty} strips the C{FOR UPDATE} clause, if one is | ||
336 | 594 | present, since it's not supported in subqueries and thre is no reason | ||
337 | 595 | to acquire a row-level lock while testing if a ResultSet is empty | ||
338 | 596 | """ | ||
339 | 597 | stream = StringIO() | ||
340 | 598 | self.addCleanup(debug, False) | ||
341 | 599 | debug(True, stream) | ||
342 | 600 | |||
343 | 601 | result = self.store.find(Foo, Foo.id == 300).for_update() | ||
344 | 602 | result.order_by(Foo.id) | ||
345 | 603 | self.assertEqual(True, result.is_empty()) | ||
346 | 604 | self.assertNotIn("FOR UPDATE", stream.getvalue()) | ||
347 | 605 | |||
348 | 572 | def test_is_empty_with_composed_key(self): | 606 | def test_is_empty_with_composed_key(self): |
349 | 573 | result = self.store.find(Link, foo_id=300, bar_id=3000) | 607 | result = self.store.find(Link, foo_id=300, bar_id=3000) |
350 | 574 | self.assertEquals(result.is_empty(), True) | 608 | self.assertEquals(result.is_empty(), True) |
351 | @@ -592,6 +626,23 @@ | |||
352 | 592 | (30, "Title 10"), | 626 | (30, "Title 10"), |
353 | 593 | ]) | 627 | ]) |
354 | 594 | 628 | ||
355 | 629 | def test_find_for_update(self): | ||
356 | 630 | stream = StringIO() | ||
357 | 631 | self.addCleanup(debug, False) | ||
358 | 632 | debug(True, stream) | ||
359 | 633 | |||
360 | 634 | result = self.store.find(Foo).for_update() | ||
361 | 635 | lst = [(foo.id, foo.title) for foo in result] | ||
362 | 636 | lst.sort() | ||
363 | 637 | self.assertEquals(lst, [ | ||
364 | 638 | (10, "Title 30"), | ||
365 | 639 | (20, "Title 20"), | ||
366 | 640 | (30, "Title 10"), | ||
367 | 641 | ]) | ||
368 | 642 | if self.__class__.__name__.startswith("SQLite"): | ||
369 | 643 | return | ||
370 | 644 | self.assertIn("FOR UPDATE", stream.getvalue()) | ||
371 | 645 | |||
372 | 595 | def test_find_from_cache(self): | 646 | def test_find_from_cache(self): |
373 | 596 | foo = self.store.get(Foo, 10) | 647 | foo = self.store.get(Foo, 10) |
374 | 597 | self.assertTrue(self.store.find(Foo, id=10).one() is foo) | 648 | self.assertTrue(self.store.find(Foo, id=10).one() is foo) |
375 | @@ -940,6 +991,11 @@ | |||
376 | 940 | def test_find_count(self): | 991 | def test_find_count(self): |
377 | 941 | self.assertEquals(self.store.find(Foo).count(), 3) | 992 | self.assertEquals(self.store.find(Foo).count(), 3) |
378 | 942 | 993 | ||
379 | 994 | def test_find_count_for_update(self): | ||
380 | 995 | result = self.store.find(Foo) | ||
381 | 996 | result.for_update() | ||
382 | 997 | self.assertRaises(FeatureError, result.count, 3) | ||
383 | 998 | |||
384 | 943 | def test_find_count_after_slice(self): | 999 | def test_find_count_after_slice(self): |
385 | 944 | """ | 1000 | """ |
386 | 945 | When we slice a ResultSet obtained after a set operation (like union), | 1001 | When we slice a ResultSet obtained after a set operation (like union), |
387 | @@ -1007,6 +1063,11 @@ | |||
388 | 1007 | def test_find_max(self): | 1063 | def test_find_max(self): |
389 | 1008 | self.assertEquals(self.store.find(Foo).max(Foo.id), 30) | 1064 | self.assertEquals(self.store.find(Foo).max(Foo.id), 30) |
390 | 1009 | 1065 | ||
391 | 1066 | def test_find_max_for_update(self): | ||
392 | 1067 | result = self.store.find(Foo) | ||
393 | 1068 | result.for_update() | ||
394 | 1069 | self.assertRaises(FeatureError, result.max, Foo.id) | ||
395 | 1070 | |||
396 | 1010 | def test_find_max_expr(self): | 1071 | def test_find_max_expr(self): |
397 | 1011 | self.assertEquals(self.store.find(Foo).max(Foo.id + 1), 31) | 1072 | self.assertEquals(self.store.find(Foo).max(Foo.id + 1), 31) |
398 | 1012 | 1073 | ||
399 | @@ -1028,6 +1089,11 @@ | |||
400 | 1028 | def test_find_min(self): | 1089 | def test_find_min(self): |
401 | 1029 | self.assertEquals(self.store.find(Foo).min(Foo.id), 10) | 1090 | self.assertEquals(self.store.find(Foo).min(Foo.id), 10) |
402 | 1030 | 1091 | ||
403 | 1092 | def test_find_min_for_update(self): | ||
404 | 1093 | result = self.store.find(Foo) | ||
405 | 1094 | result.for_update() | ||
406 | 1095 | self.assertRaises(FeatureError, result.min, Foo.id) | ||
407 | 1096 | |||
408 | 1031 | def test_find_min_expr(self): | 1097 | def test_find_min_expr(self): |
409 | 1032 | self.assertEquals(self.store.find(Foo).min(Foo.id - 1), 9) | 1098 | self.assertEquals(self.store.find(Foo).min(Foo.id - 1), 9) |
410 | 1033 | 1099 | ||
411 | @@ -1049,6 +1115,11 @@ | |||
412 | 1049 | def test_find_avg(self): | 1115 | def test_find_avg(self): |
413 | 1050 | self.assertEquals(self.store.find(Foo).avg(Foo.id), 20) | 1116 | self.assertEquals(self.store.find(Foo).avg(Foo.id), 20) |
414 | 1051 | 1117 | ||
415 | 1118 | def test_find_avg_for_update(self): | ||
416 | 1119 | result = self.store.find(Foo) | ||
417 | 1120 | result.for_update() | ||
418 | 1121 | self.assertRaises(FeatureError, result.avg, Foo.id) | ||
419 | 1122 | |||
420 | 1052 | def test_find_avg_expr(self): | 1123 | def test_find_avg_expr(self): |
421 | 1053 | self.assertEquals(self.store.find(Foo).avg(Foo.id + 10), 30) | 1124 | self.assertEquals(self.store.find(Foo).avg(Foo.id + 10), 30) |
422 | 1054 | 1125 | ||
423 | @@ -1062,6 +1133,11 @@ | |||
424 | 1062 | def test_find_sum(self): | 1133 | def test_find_sum(self): |
425 | 1063 | self.assertEquals(self.store.find(Foo).sum(Foo.id), 60) | 1134 | self.assertEquals(self.store.find(Foo).sum(Foo.id), 60) |
426 | 1064 | 1135 | ||
427 | 1136 | def test_find_sum_for_update(self): | ||
428 | 1137 | result = self.store.find(Foo) | ||
429 | 1138 | result.for_update() | ||
430 | 1139 | self.assertRaises(FeatureError, result.sum, Foo.id) | ||
431 | 1140 | |||
432 | 1065 | def test_find_sum_expr(self): | 1141 | def test_find_sum_expr(self): |
433 | 1066 | self.assertEquals(self.store.find(Foo).sum(Foo.id * 2), 120) | 1142 | self.assertEquals(self.store.find(Foo).sum(Foo.id * 2), 120) |
434 | 1067 | 1143 | ||
435 | @@ -1546,6 +1622,11 @@ | |||
436 | 1546 | result = list(result) | 1622 | result = list(result) |
437 | 1547 | self.assertEquals(result, [(2L, 2L), (2L, 2L), (2L, 3L), (3L, 6L)]) | 1623 | self.assertEquals(result, [(2L, 2L), (2L, 2L), (2L, 3L), (3L, 6L)]) |
438 | 1548 | 1624 | ||
439 | 1625 | def test_find_group_by_for_update(self): | ||
440 | 1626 | result = self.store.find((Count(FooValue.id), Sum(FooValue.value1))) | ||
441 | 1627 | result.group_by(FooValue.value2) | ||
442 | 1628 | self.assertRaises(FeatureError, result.for_update) | ||
443 | 1629 | |||
444 | 1549 | def test_find_group_by_table(self): | 1630 | def test_find_group_by_table(self): |
445 | 1550 | result = self.store.find( | 1631 | result = self.store.find( |
446 | 1551 | (Sum(FooValue.value2), Foo), Foo.id == FooValue.foo_id) | 1632 | (Sum(FooValue.value2), Foo), Foo.id == FooValue.foo_id) |
447 | @@ -2271,6 +2352,13 @@ | |||
448 | 2271 | self.assertEquals(foo.id, 20) | 2352 | self.assertEquals(foo.id, 20) |
449 | 2272 | self.assertEquals(foo.title, "Title 20") | 2353 | self.assertEquals(foo.title, "Title 20") |
450 | 2273 | 2354 | ||
451 | 2355 | def test_sub_select_for_update(self): | ||
452 | 2356 | foo = self.store.find(Foo, Foo.id == Select(SQL("20")) | ||
453 | 2357 | ).for_update().one() | ||
454 | 2358 | self.assertTrue(foo) | ||
455 | 2359 | self.assertEquals(foo.id, 20) | ||
456 | 2360 | self.assertEquals(foo.title, "Title 20") | ||
457 | 2361 | |||
458 | 2274 | def test_cache_has_improper_object(self): | 2362 | def test_cache_has_improper_object(self): |
459 | 2275 | foo = self.store.get(Foo, 20) | 2363 | foo = self.store.get(Foo, 20) |
460 | 2276 | self.store.remove(foo) | 2364 | self.store.remove(foo) |
461 | @@ -5472,6 +5560,12 @@ | |||
462 | 5472 | (30, "Title 10"), | 5560 | (30, "Title 10"), |
463 | 5473 | ]) | 5561 | ]) |
464 | 5474 | 5562 | ||
465 | 5563 | def test_result_union_for_update(self): | ||
466 | 5564 | result1 = self.store.find(Foo, id=30).for_update() | ||
467 | 5565 | result2 = self.store.find(Foo, id=10) | ||
468 | 5566 | self.assertRaises(FeatureError, result1.union, result2) | ||
469 | 5567 | self.assertRaises(FeatureError, result2.union, result1) | ||
470 | 5568 | |||
471 | 5475 | def test_result_union_duplicated(self): | 5569 | def test_result_union_duplicated(self): |
472 | 5476 | result1 = self.store.find(Foo, id=30) | 5570 | result1 = self.store.find(Foo, id=30) |
473 | 5477 | result2 = self.store.find(Foo, id=30) | 5571 | result2 = self.store.find(Foo, id=30) |
474 | @@ -5544,6 +5638,12 @@ | |||
475 | 5544 | (30, "Title 10"), | 5638 | (30, "Title 10"), |
476 | 5545 | ]) | 5639 | ]) |
477 | 5546 | 5640 | ||
478 | 5641 | def test_result_difference_for_update(self): | ||
479 | 5642 | result1 = self.store.find(Foo).for_update() | ||
480 | 5643 | result2 = self.store.find(Foo, id=10) | ||
481 | 5644 | self.assertRaises(FeatureError, result1.difference, result2) | ||
482 | 5645 | self.assertRaises(FeatureError, result2.difference, result1) | ||
483 | 5646 | |||
484 | 5547 | def test_result_difference_with_empty(self): | 5647 | def test_result_difference_with_empty(self): |
485 | 5548 | if self.__class__.__name__.startswith("MySQL"): | 5648 | if self.__class__.__name__.startswith("MySQL"): |
486 | 5549 | return | 5649 | return |
487 | @@ -5605,6 +5705,12 @@ | |||
488 | 5605 | (30, "Title 10"), | 5705 | (30, "Title 10"), |
489 | 5606 | ]) | 5706 | ]) |
490 | 5607 | 5707 | ||
491 | 5708 | def test_result_intersection_for_update(self): | ||
492 | 5709 | result1 = self.store.find(Foo).for_update() | ||
493 | 5710 | result2 = self.store.find(Foo, Foo.id.is_in((10, 30))) | ||
494 | 5711 | self.assertRaises(FeatureError, result1.intersection, result2) | ||
495 | 5712 | self.assertRaises(FeatureError, result2.intersection, result1) | ||
496 | 5713 | |||
497 | 5608 | def test_result_intersection_with_empty(self): | 5714 | def test_result_intersection_with_empty(self): |
498 | 5609 | if self.__class__.__name__.startswith("MySQL"): | 5715 | if self.__class__.__name__.startswith("MySQL"): |
499 | 5610 | return | 5716 | return |
500 | @@ -6048,6 +6154,10 @@ | |||
501 | 6048 | self.assertEquals(self.result.order_by(Foo.title), self.result) | 6154 | self.assertEquals(self.result.order_by(Foo.title), self.result) |
502 | 6049 | self.assertEquals(self.empty.order_by(Foo.title), self.empty) | 6155 | self.assertEquals(self.empty.order_by(Foo.title), self.empty) |
503 | 6050 | 6156 | ||
504 | 6157 | def test_for_update(self): | ||
505 | 6158 | self.assertEquals(self.result.for_update(), self.result) | ||
506 | 6159 | self.assertEquals(self.empty.for_update(), self.empty) | ||
507 | 6160 | |||
508 | 6051 | def test_remove(self): | 6161 | def test_remove(self): |
509 | 6052 | self.assertEquals(self.result.remove(), 0) | 6162 | self.assertEquals(self.result.remove(), 0) |
510 | 6053 | self.assertEquals(self.empty.remove(), 0) | 6163 | self.assertEquals(self.empty.remove(), 0) |
Edoardo, thanks for this branch! Before we spend time reviewing the
changes, have you signed the Canonical copyright assignment forms?
This process will need to be completed before we can accept changes.