Merge lp:~tdfischer/zeitgeist/timerange-deletion-api into lp:~zeitgeist/zeitgeist/bluebird
- timerange-deletion-api
- Merge into bluebird
Proposed by
Trever Fischer
Status: | Needs review | ||||
---|---|---|---|---|---|
Proposed branch: | lp:~tdfischer/zeitgeist/timerange-deletion-api | ||||
Merge into: | lp:~zeitgeist/zeitgeist/bluebird | ||||
Prerequisite: | lp:~tdfischer/zeitgeist/common-where | ||||
Diff against target: |
295 lines (+139/-106) 4 files modified
src/db-reader.vala (+81/-106) src/engine.vala (+40/-0) src/remote.vala (+7/-0) src/zeitgeist-daemon.vala (+11/-0) |
||||
To merge this branch: | bzr merge lp:~tdfischer/zeitgeist/timerange-deletion-api | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Siegfried Gevatter | Needs Fixing | ||
Review via email: mp+97272@code.launchpad.net |
Commit message
Description of the change
Fixes #799531
To post a comment you must log in.
- 423. By Trever Fischer
-
--rolling_
face_on_ keyboard
Revision history for this message
Trever Fischer (tdfischer) wrote : | # |
As noted in the original bug, simply querying all the event ids in a time range and then handing it to delete_events causes the SQL error described: "Too many variables"
Revision history for this message
Siegfried Gevatter (rainct) wrote : | # |
(Follow up from IRC: the "too many variables" bug has already been fixed)
Unmerged revisions
- 423. By Trever Fischer
-
--rolling_
face_on_ keyboard - 422. By Trever Fischer
-
Implement timerange deletion
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/db-reader.vala' | |||
2 | --- src/db-reader.vala 2012-03-17 14:47:09 +0000 | |||
3 | +++ src/db-reader.vala 2012-03-19 17:30:32 +0000 | |||
4 | @@ -170,112 +170,7 @@ | |||
5 | 170 | where_sql = "WHERE " + where.get_sql_conditions (); | 170 | where_sql = "WHERE " + where.get_sql_conditions (); |
6 | 171 | } | 171 | } |
7 | 172 | 172 | ||
114 | 173 | switch (result_type) | 173 | sql += group_clause (result_type, where_sql); |
9 | 174 | { | ||
10 | 175 | case ResultType.MOST_RECENT_EVENTS: | ||
11 | 176 | sql += where_sql + " ORDER BY timestamp DESC"; | ||
12 | 177 | break; | ||
13 | 178 | case ResultType.LEAST_RECENT_EVENTS: | ||
14 | 179 | sql += where_sql + " ORDER BY timestamp ASC"; | ||
15 | 180 | break; | ||
16 | 181 | case ResultType.MOST_RECENT_EVENT_ORIGIN: | ||
17 | 182 | sql += group_and_sort ("origin", where_sql, false); | ||
18 | 183 | break; | ||
19 | 184 | case ResultType.LEAST_RECENT_EVENT_ORIGIN: | ||
20 | 185 | sql += group_and_sort ("origin", where_sql, true); | ||
21 | 186 | break; | ||
22 | 187 | case ResultType.MOST_POPULAR_EVENT_ORIGIN: | ||
23 | 188 | sql += group_and_sort ("origin", where_sql, false, false); | ||
24 | 189 | break; | ||
25 | 190 | case ResultType.LEAST_POPULAR_EVENT_ORIGIN: | ||
26 | 191 | sql += group_and_sort ("origin", where_sql, true, true); | ||
27 | 192 | break; | ||
28 | 193 | case ResultType.MOST_RECENT_SUBJECTS: | ||
29 | 194 | sql += group_and_sort ("subj_id", where_sql, false); | ||
30 | 195 | break; | ||
31 | 196 | case ResultType.LEAST_RECENT_SUBJECTS: | ||
32 | 197 | sql += group_and_sort ("subj_id", where_sql, true); | ||
33 | 198 | break; | ||
34 | 199 | case ResultType.MOST_POPULAR_SUBJECTS: | ||
35 | 200 | sql += group_and_sort ("subj_id", where_sql, false, false); | ||
36 | 201 | break; | ||
37 | 202 | case ResultType.LEAST_POPULAR_SUBJECTS: | ||
38 | 203 | sql += group_and_sort ("subj_id", where_sql, true, true); | ||
39 | 204 | break; | ||
40 | 205 | case ResultType.MOST_RECENT_CURRENT_URI: | ||
41 | 206 | sql += group_and_sort ("subj_id_current", where_sql, false); | ||
42 | 207 | break; | ||
43 | 208 | case ResultType.LEAST_RECENT_CURRENT_URI: | ||
44 | 209 | sql += group_and_sort ("subj_id_current", where_sql, true); | ||
45 | 210 | break; | ||
46 | 211 | case ResultType.MOST_POPULAR_CURRENT_URI: | ||
47 | 212 | sql += group_and_sort ("subj_id_current", where_sql, | ||
48 | 213 | false, false); | ||
49 | 214 | break; | ||
50 | 215 | case ResultType.LEAST_POPULAR_CURRENT_URI: | ||
51 | 216 | sql += group_and_sort ("subj_id_current", where_sql, | ||
52 | 217 | true, true); | ||
53 | 218 | break; | ||
54 | 219 | case ResultType.MOST_RECENT_ACTOR: | ||
55 | 220 | sql += group_and_sort ("actor", where_sql, false); | ||
56 | 221 | break; | ||
57 | 222 | case ResultType.LEAST_RECENT_ACTOR: | ||
58 | 223 | sql += group_and_sort ("actor", where_sql, true); | ||
59 | 224 | break; | ||
60 | 225 | case ResultType.MOST_POPULAR_ACTOR: | ||
61 | 226 | sql += group_and_sort ("actor", where_sql, false, false); | ||
62 | 227 | break; | ||
63 | 228 | case ResultType.LEAST_POPULAR_ACTOR: | ||
64 | 229 | sql += group_and_sort ("actor", where_sql, true, true); | ||
65 | 230 | break; | ||
66 | 231 | case ResultType.OLDEST_ACTOR: | ||
67 | 232 | sql += group_and_sort ("actor", where_sql, true, null, "min"); | ||
68 | 233 | break; | ||
69 | 234 | case ResultType.MOST_RECENT_ORIGIN: | ||
70 | 235 | sql += group_and_sort ("subj_origin", where_sql, false); | ||
71 | 236 | break; | ||
72 | 237 | case ResultType.LEAST_RECENT_ORIGIN: | ||
73 | 238 | sql += group_and_sort ("subj_origin", where_sql, true); | ||
74 | 239 | break; | ||
75 | 240 | case ResultType.MOST_POPULAR_ORIGIN: | ||
76 | 241 | sql += group_and_sort ("subj_origin", where_sql, false, false); | ||
77 | 242 | break; | ||
78 | 243 | case ResultType.LEAST_POPULAR_ORIGIN: | ||
79 | 244 | sql += group_and_sort ("subj_origin", where_sql, true, true); | ||
80 | 245 | break; | ||
81 | 246 | case ResultType.MOST_RECENT_SUBJECT_INTERPRETATION: | ||
82 | 247 | sql += group_and_sort ("subj_interpretation", where_sql, false); | ||
83 | 248 | break; | ||
84 | 249 | case ResultType.LEAST_RECENT_SUBJECT_INTERPRETATION: | ||
85 | 250 | sql += group_and_sort ("subj_interpretation", where_sql, true); | ||
86 | 251 | break; | ||
87 | 252 | case ResultType.MOST_POPULAR_SUBJECT_INTERPRETATION: | ||
88 | 253 | sql += group_and_sort ("subj_interpretation", where_sql, | ||
89 | 254 | false, false); | ||
90 | 255 | break; | ||
91 | 256 | case ResultType.LEAST_POPULAR_SUBJECT_INTERPRETATION: | ||
92 | 257 | sql += group_and_sort ("subj_interpretation", where_sql, | ||
93 | 258 | true, true); | ||
94 | 259 | break; | ||
95 | 260 | case ResultType.MOST_RECENT_MIMETYPE: | ||
96 | 261 | sql += group_and_sort ("subj_mimetype", where_sql, false); | ||
97 | 262 | break; | ||
98 | 263 | case ResultType.LEAST_RECENT_MIMETYPE: | ||
99 | 264 | sql += group_and_sort ("subj_mimetype", where_sql, true); | ||
100 | 265 | break; | ||
101 | 266 | case ResultType.MOST_POPULAR_MIMETYPE: | ||
102 | 267 | sql += group_and_sort ("subj_mimetype", where_sql, | ||
103 | 268 | false, false); | ||
104 | 269 | break; | ||
105 | 270 | case ResultType.LEAST_POPULAR_MIMETYPE: | ||
106 | 271 | sql += group_and_sort ("subj_mimetype", where_sql, | ||
107 | 272 | true, true); | ||
108 | 273 | break; | ||
109 | 274 | default: | ||
110 | 275 | string error_message = "Invalid ResultType."; | ||
111 | 276 | warning (error_message); | ||
112 | 277 | throw new EngineError.INVALID_ARGUMENT (error_message); | ||
113 | 278 | } | ||
115 | 279 | 174 | ||
116 | 280 | int rc; | 175 | int rc; |
117 | 281 | Sqlite.Statement stmt; | 176 | Sqlite.Statement stmt; |
118 | @@ -576,6 +471,86 @@ | |||
119 | 576 | database.close (); | 471 | database.close (); |
120 | 577 | } | 472 | } |
121 | 578 | 473 | ||
122 | 474 | public string group_clause (uint type, string where_sql) throws EngineError | ||
123 | 475 | { | ||
124 | 476 | switch (type) | ||
125 | 477 | { | ||
126 | 478 | case ResultType.MOST_RECENT_EVENTS: | ||
127 | 479 | return " ORDER BY timestamp DESC"; | ||
128 | 480 | case ResultType.LEAST_RECENT_EVENTS: | ||
129 | 481 | return " ORDER BY timestamp ASC"; | ||
130 | 482 | case ResultType.MOST_RECENT_EVENT_ORIGIN: | ||
131 | 483 | return group_and_sort ("origin", where_sql, false); | ||
132 | 484 | case ResultType.LEAST_RECENT_EVENT_ORIGIN: | ||
133 | 485 | return group_and_sort ("origin", where_sql, true); | ||
134 | 486 | case ResultType.MOST_POPULAR_EVENT_ORIGIN: | ||
135 | 487 | return group_and_sort ("origin", where_sql, false, false); | ||
136 | 488 | case ResultType.LEAST_POPULAR_EVENT_ORIGIN: | ||
137 | 489 | return group_and_sort ("origin", where_sql, true, true); | ||
138 | 490 | case ResultType.MOST_RECENT_SUBJECTS: | ||
139 | 491 | return group_and_sort ("subj_id", where_sql, false); | ||
140 | 492 | case ResultType.LEAST_RECENT_SUBJECTS: | ||
141 | 493 | return group_and_sort ("subj_id", where_sql, true); | ||
142 | 494 | case ResultType.MOST_POPULAR_SUBJECTS: | ||
143 | 495 | return group_and_sort ("subj_id", where_sql, false, false); | ||
144 | 496 | case ResultType.LEAST_POPULAR_SUBJECTS: | ||
145 | 497 | return group_and_sort ("subj_id", where_sql, true, true); | ||
146 | 498 | case ResultType.MOST_RECENT_CURRENT_URI: | ||
147 | 499 | return group_and_sort ("subj_id_current", where_sql, false); | ||
148 | 500 | case ResultType.LEAST_RECENT_CURRENT_URI: | ||
149 | 501 | return group_and_sort ("subj_id_current", where_sql, true); | ||
150 | 502 | break; | ||
151 | 503 | case ResultType.MOST_POPULAR_CURRENT_URI: | ||
152 | 504 | return group_and_sort ("subj_id_current", where_sql, | ||
153 | 505 | false, false); | ||
154 | 506 | case ResultType.LEAST_POPULAR_CURRENT_URI: | ||
155 | 507 | return group_and_sort ("subj_id_current", where_sql, | ||
156 | 508 | true, true); | ||
157 | 509 | case ResultType.MOST_RECENT_ACTOR: | ||
158 | 510 | return group_and_sort ("actor", where_sql, false); | ||
159 | 511 | case ResultType.LEAST_RECENT_ACTOR: | ||
160 | 512 | return group_and_sort ("actor", where_sql, true); | ||
161 | 513 | case ResultType.MOST_POPULAR_ACTOR: | ||
162 | 514 | return group_and_sort ("actor", where_sql, false, false); | ||
163 | 515 | case ResultType.LEAST_POPULAR_ACTOR: | ||
164 | 516 | return group_and_sort ("actor", where_sql, true, true); | ||
165 | 517 | case ResultType.OLDEST_ACTOR: | ||
166 | 518 | return group_and_sort ("actor", where_sql, true, null, "min"); | ||
167 | 519 | case ResultType.MOST_RECENT_ORIGIN: | ||
168 | 520 | return group_and_sort ("subj_origin", where_sql, false); | ||
169 | 521 | case ResultType.LEAST_RECENT_ORIGIN: | ||
170 | 522 | return group_and_sort ("subj_origin", where_sql, true); | ||
171 | 523 | case ResultType.MOST_POPULAR_ORIGIN: | ||
172 | 524 | return group_and_sort ("subj_origin", where_sql, false, false); | ||
173 | 525 | case ResultType.LEAST_POPULAR_ORIGIN: | ||
174 | 526 | return group_and_sort ("subj_origin", where_sql, true, true); | ||
175 | 527 | case ResultType.MOST_RECENT_SUBJECT_INTERPRETATION: | ||
176 | 528 | return group_and_sort ("subj_interpretation", where_sql, false); | ||
177 | 529 | case ResultType.LEAST_RECENT_SUBJECT_INTERPRETATION: | ||
178 | 530 | return group_and_sort ("subj_interpretation", where_sql, true); | ||
179 | 531 | case ResultType.MOST_POPULAR_SUBJECT_INTERPRETATION: | ||
180 | 532 | return group_and_sort ("subj_interpretation", where_sql, | ||
181 | 533 | false, false); | ||
182 | 534 | case ResultType.LEAST_POPULAR_SUBJECT_INTERPRETATION: | ||
183 | 535 | return group_and_sort ("subj_interpretation", where_sql, | ||
184 | 536 | true, true); | ||
185 | 537 | case ResultType.MOST_RECENT_MIMETYPE: | ||
186 | 538 | return group_and_sort ("subj_mimetype", where_sql, false); | ||
187 | 539 | case ResultType.LEAST_RECENT_MIMETYPE: | ||
188 | 540 | return group_and_sort ("subj_mimetype", where_sql, true); | ||
189 | 541 | case ResultType.MOST_POPULAR_MIMETYPE: | ||
190 | 542 | return group_and_sort ("subj_mimetype", where_sql, | ||
191 | 543 | false, false); | ||
192 | 544 | case ResultType.LEAST_POPULAR_MIMETYPE: | ||
193 | 545 | return group_and_sort ("subj_mimetype", where_sql, | ||
194 | 546 | true, true); | ||
195 | 547 | default: | ||
196 | 548 | string error_message = "Invalid ResultType."; | ||
197 | 549 | warning (error_message); | ||
198 | 550 | throw new EngineError.INVALID_ARGUMENT (error_message); | ||
199 | 551 | } | ||
200 | 552 | } | ||
201 | 553 | |||
202 | 579 | // Used by find_event_ids | 554 | // Used by find_event_ids |
203 | 580 | private string group_and_sort (string field, string where_sql, | 555 | private string group_and_sort (string field, string where_sql, |
204 | 581 | bool time_asc=false, bool? count_asc=null, | 556 | bool time_asc=false, bool? count_asc=null, |
205 | 582 | 557 | ||
206 | === modified file 'src/engine.vala' | |||
207 | --- src/engine.vala 2012-03-14 14:26:11 +0000 | |||
208 | +++ src/engine.vala 2012-03-19 17:30:32 +0000 | |||
209 | @@ -277,6 +277,46 @@ | |||
210 | 277 | return event.id; | 277 | return event.id; |
211 | 278 | } | 278 | } |
212 | 279 | 279 | ||
213 | 280 | public uint32[] delete_events_in_time_range (TimeRange range, | ||
214 | 281 | GenericArray<Event> event_templates, | ||
215 | 282 | uint storage_state, uint max_events, uint result_type, | ||
216 | 283 | BusName? sender=null) throws EngineError | ||
217 | 284 | { | ||
218 | 285 | uint32[] event_ids = find_event_ids (range, event_templates, | ||
219 | 286 | storage_state, max_events, result_type); | ||
220 | 287 | |||
221 | 288 | WhereClause where = get_where_clause_for_query (range, event_templates, | ||
222 | 289 | storage_state, sender); | ||
223 | 290 | |||
224 | 291 | string sql = "DELETE FROM event_view "; | ||
225 | 292 | string where_sql = ""; | ||
226 | 293 | if (!where.is_empty ()) | ||
227 | 294 | { | ||
228 | 295 | where_sql = "WHERE " + where.get_sql_conditions (); | ||
229 | 296 | } | ||
230 | 297 | |||
231 | 298 | sql += group_clause (result_type, where_sql); | ||
232 | 299 | |||
233 | 300 | if (max_events > 0) | ||
234 | 301 | sql += " LIMIT %u".printf(max_events); | ||
235 | 302 | |||
236 | 303 | int rc; | ||
237 | 304 | Sqlite.Statement stmt; | ||
238 | 305 | |||
239 | 306 | rc = db.prepare_v2 (sql, -1, out stmt); | ||
240 | 307 | database.assert_query_success(rc, "SQL error"); | ||
241 | 308 | |||
242 | 309 | var arguments = where.get_bind_arguments (); | ||
243 | 310 | for (int i = 0; i < arguments.length; ++i) | ||
244 | 311 | stmt.bind_text (i + 1, arguments[i]); | ||
245 | 312 | |||
246 | 313 | #if EXPLAIN_QUERIES | ||
247 | 314 | database.explain_query (stmt); | ||
248 | 315 | #endif | ||
249 | 316 | |||
250 | 317 | return event_ids; | ||
251 | 318 | } | ||
252 | 319 | |||
253 | 280 | public TimeRange? delete_events (uint32[] event_ids, BusName? sender) | 320 | public TimeRange? delete_events (uint32[] event_ids, BusName? sender) |
254 | 281 | throws EngineError | 321 | throws EngineError |
255 | 282 | requires (event_ids.length > 0) | 322 | requires (event_ids.length > 0) |
256 | 283 | 323 | ||
257 | === modified file 'src/remote.vala' | |||
258 | --- src/remote.vala 2012-02-10 17:03:50 +0000 | |||
259 | +++ src/remote.vala 2012-03-19 17:30:32 +0000 | |||
260 | @@ -38,6 +38,13 @@ | |||
261 | 38 | BusName sender | 38 | BusName sender |
262 | 39 | ) throws Error; | 39 | ) throws Error; |
263 | 40 | 40 | ||
264 | 41 | public abstract uint32[] delete_events_in_time_range ( | ||
265 | 42 | [DBus (signature = "(xx)")] Variant time_range, | ||
266 | 43 | [DBus (signature = "a(asaasay)")] Variant event_templates, | ||
267 | 44 | uint storage_state, uint num_events, uint result_type, | ||
268 | 45 | BusName sender | ||
269 | 46 | ) throws Error; | ||
270 | 47 | |||
271 | 41 | public abstract uint32[] find_event_ids ( | 48 | public abstract uint32[] find_event_ids ( |
272 | 42 | [DBus (signature = "(xx)")] Variant time_range, | 49 | [DBus (signature = "(xx)")] Variant time_range, |
273 | 43 | [DBus (signature = "a(asaasay)")] Variant event_templates, | 50 | [DBus (signature = "a(asaasay)")] Variant event_templates, |
274 | 44 | 51 | ||
275 | === modified file 'src/zeitgeist-daemon.vala' | |||
276 | --- src/zeitgeist-daemon.vala 2012-03-01 14:47:30 +0000 | |||
277 | +++ src/zeitgeist-daemon.vala 2012-03-19 17:30:32 +0000 | |||
278 | @@ -205,6 +205,17 @@ | |||
279 | 205 | return event_ids; | 205 | return event_ids; |
280 | 206 | } | 206 | } |
281 | 207 | 207 | ||
282 | 208 | public uint32[] delete_events_in_time_range (Variant range, | ||
283 | 209 | Variant event_templates, | ||
284 | 210 | uint storage_state, uint num_events, uint result_type, | ||
285 | 211 | BusName sender) throws Error | ||
286 | 212 | { | ||
287 | 213 | uint32[] event_ids = engine.delete_events_in_time_range (new TimeRange.from_variant(range), | ||
288 | 214 | Events.from_variant (event_templates), | ||
289 | 215 | storage_state, num_events, result_type, sender); | ||
290 | 216 | return event_ids; | ||
291 | 217 | } | ||
292 | 218 | |||
293 | 208 | public Variant delete_events (uint32[] event_ids, BusName sender) | 219 | public Variant delete_events (uint32[] event_ids, BusName sender) |
294 | 209 | throws Error | 220 | throws Error |
295 | 210 | { | 221 | { |
Hi Trever,
First of all, thank you for working on this!
As commented on IRC, I don't really see a point for storage_state, num_events and result_type in the event deletion method. The original proposal just had time_range and event_templates.
(I'm also unconvinced that this methods performance is critical enough to warrant the complexity of duplicating the query. I think I may prefer a FindEventIds+ DeleteEvents implementation, like Seif proposed.)