Merge lp:~tdfischer/zeitgeist/timerange-deletion-api into lp:~zeitgeist/zeitgeist/bluebird

[Trever Fischer]

Fixes #799531

[lp:~tdfischer/zeitgeist/timerange-deletion-api]

423. By Trever Fischer on 2012-03-19

--rolling_face_on_keyboard

[Siegfried Gevatter (rainct)]

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.)

review: Needs Fixing

[Trever Fischer (tdfischer)]

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"

[Siegfried Gevatter (rainct)]

(Follow up from IRC: the "too many variables" bug has already been fixed)

Unmerged revisions

423. By Trever Fischer on 2012-03-19

--rolling_face_on_keyboard

422. By Trever Fischer on 2012-03-19

Implement timerange deletion

=== modified file 'src/db-reader.vala'
--- src/db-reader.vala	2012-03-17 14:47:09 +0000
+++ src/db-reader.vala	2012-03-19 17:30:32 +0000
@@ -170,112 +170,7 @@
             where_sql = "WHERE " + where.get_sql_conditions ();
         }
 
-        switch (result_type)
-        {
-            case ResultType.MOST_RECENT_EVENTS:
-                sql += where_sql + " ORDER BY timestamp DESC";
-                break;
-            case ResultType.LEAST_RECENT_EVENTS:
-                sql += where_sql + " ORDER BY timestamp ASC";
-                break;
-            case ResultType.MOST_RECENT_EVENT_ORIGIN:
-                sql += group_and_sort ("origin", where_sql, false);
-                break;
-            case ResultType.LEAST_RECENT_EVENT_ORIGIN:
-                sql += group_and_sort ("origin", where_sql, true);
-                break;
-            case ResultType.MOST_POPULAR_EVENT_ORIGIN:
-                sql += group_and_sort ("origin", where_sql, false, false);
-                break;
-            case ResultType.LEAST_POPULAR_EVENT_ORIGIN:
-                sql += group_and_sort ("origin", where_sql, true, true);
-                break;
-            case ResultType.MOST_RECENT_SUBJECTS:
-                sql += group_and_sort ("subj_id", where_sql, false);
-                break;
-            case ResultType.LEAST_RECENT_SUBJECTS:
-                sql += group_and_sort ("subj_id", where_sql, true);
-                break;
-            case ResultType.MOST_POPULAR_SUBJECTS:
-                sql += group_and_sort ("subj_id", where_sql, false, false);
-                break;
-            case ResultType.LEAST_POPULAR_SUBJECTS:
-                sql += group_and_sort ("subj_id", where_sql, true, true);
-                break;
-            case ResultType.MOST_RECENT_CURRENT_URI:
-                sql += group_and_sort ("subj_id_current", where_sql, false);
-                break;
-            case ResultType.LEAST_RECENT_CURRENT_URI:
-                sql += group_and_sort ("subj_id_current", where_sql, true);
-                break;
-            case ResultType.MOST_POPULAR_CURRENT_URI:
-                sql += group_and_sort ("subj_id_current", where_sql,
-                    false, false);
-                break;
-            case ResultType.LEAST_POPULAR_CURRENT_URI:
-                sql += group_and_sort ("subj_id_current", where_sql,
-                    true, true);
-                break;
-            case ResultType.MOST_RECENT_ACTOR:
-                sql += group_and_sort ("actor", where_sql, false);
-                break;
-            case ResultType.LEAST_RECENT_ACTOR:
-                sql += group_and_sort ("actor", where_sql, true);
-                break;
-            case ResultType.MOST_POPULAR_ACTOR:
-                sql += group_and_sort ("actor", where_sql, false, false);
-                break;
-            case ResultType.LEAST_POPULAR_ACTOR:
-                sql += group_and_sort ("actor", where_sql, true, true);
-                break;
-            case ResultType.OLDEST_ACTOR:
-                sql += group_and_sort ("actor", where_sql, true, null, "min");
-                break;
-            case ResultType.MOST_RECENT_ORIGIN:
-                sql += group_and_sort ("subj_origin", where_sql, false);
-                break;
-            case ResultType.LEAST_RECENT_ORIGIN:
-                sql += group_and_sort ("subj_origin", where_sql, true);
-                break;
-            case ResultType.MOST_POPULAR_ORIGIN:
-                sql += group_and_sort ("subj_origin", where_sql, false, false);
-                break;
-            case ResultType.LEAST_POPULAR_ORIGIN:
-                sql += group_and_sort ("subj_origin", where_sql, true, true);
-                break;
-            case ResultType.MOST_RECENT_SUBJECT_INTERPRETATION:
-                sql += group_and_sort ("subj_interpretation", where_sql, false);
-                break;
-            case ResultType.LEAST_RECENT_SUBJECT_INTERPRETATION:
-                sql += group_and_sort ("subj_interpretation", where_sql, true);
-                break;
-            case ResultType.MOST_POPULAR_SUBJECT_INTERPRETATION:
-                sql += group_and_sort ("subj_interpretation", where_sql,
-                    false, false);
-                break;
-            case ResultType.LEAST_POPULAR_SUBJECT_INTERPRETATION:
-                sql += group_and_sort ("subj_interpretation", where_sql,
-                    true, true);
-                break;
-            case ResultType.MOST_RECENT_MIMETYPE:
-                sql += group_and_sort ("subj_mimetype", where_sql, false);
-                break;
-            case ResultType.LEAST_RECENT_MIMETYPE:
-                sql += group_and_sort ("subj_mimetype", where_sql, true);
-                break;
-            case ResultType.MOST_POPULAR_MIMETYPE:
-                sql += group_and_sort ("subj_mimetype", where_sql,
-                    false, false);
-                break;
-            case ResultType.LEAST_POPULAR_MIMETYPE:
-                sql += group_and_sort ("subj_mimetype", where_sql,
-                    true, true);
-                break;
-            default:
-                string error_message = "Invalid ResultType.";
-                warning (error_message);
-                throw new EngineError.INVALID_ARGUMENT (error_message);
-        }
+        sql += group_clause (result_type, where_sql);
 
         int rc;
         Sqlite.Statement stmt;
@@ -576,6 +471,86 @@
         database.close ();
     }
 
+    public string group_clause (uint type, string where_sql) throws EngineError
+    {
+        switch (type)
+        {
+            case ResultType.MOST_RECENT_EVENTS:
+                return " ORDER BY timestamp DESC";
+            case ResultType.LEAST_RECENT_EVENTS:
+                return " ORDER BY timestamp ASC";
+            case ResultType.MOST_RECENT_EVENT_ORIGIN:
+                return group_and_sort ("origin", where_sql, false);
+            case ResultType.LEAST_RECENT_EVENT_ORIGIN:
+                return group_and_sort ("origin", where_sql, true);
+            case ResultType.MOST_POPULAR_EVENT_ORIGIN:
+                return group_and_sort ("origin", where_sql, false, false);
+            case ResultType.LEAST_POPULAR_EVENT_ORIGIN:
+                return group_and_sort ("origin", where_sql, true, true);
+            case ResultType.MOST_RECENT_SUBJECTS:
+                return group_and_sort ("subj_id", where_sql, false);
+            case ResultType.LEAST_RECENT_SUBJECTS:
+                return group_and_sort ("subj_id", where_sql, true);
+            case ResultType.MOST_POPULAR_SUBJECTS:
+                return group_and_sort ("subj_id", where_sql, false, false);
+            case ResultType.LEAST_POPULAR_SUBJECTS:
+                return group_and_sort ("subj_id", where_sql, true, true);
+            case ResultType.MOST_RECENT_CURRENT_URI:
+                return group_and_sort ("subj_id_current", where_sql, false);
+            case ResultType.LEAST_RECENT_CURRENT_URI:
+                return group_and_sort ("subj_id_current", where_sql, true);
+                break;
+            case ResultType.MOST_POPULAR_CURRENT_URI:
+                return group_and_sort ("subj_id_current", where_sql,
+                    false, false);
+            case ResultType.LEAST_POPULAR_CURRENT_URI:
+                return group_and_sort ("subj_id_current", where_sql,
+                    true, true);
+            case ResultType.MOST_RECENT_ACTOR:
+                return group_and_sort ("actor", where_sql, false);
+            case ResultType.LEAST_RECENT_ACTOR:
+                return group_and_sort ("actor", where_sql, true);
+            case ResultType.MOST_POPULAR_ACTOR:
+                return group_and_sort ("actor", where_sql, false, false);
+            case ResultType.LEAST_POPULAR_ACTOR:
+                return group_and_sort ("actor", where_sql, true, true);
+            case ResultType.OLDEST_ACTOR:
+                return group_and_sort ("actor", where_sql, true, null, "min");
+            case ResultType.MOST_RECENT_ORIGIN:
+                return group_and_sort ("subj_origin", where_sql, false);
+            case ResultType.LEAST_RECENT_ORIGIN:
+                return group_and_sort ("subj_origin", where_sql, true);
+            case ResultType.MOST_POPULAR_ORIGIN:
+                return group_and_sort ("subj_origin", where_sql, false, false);
+            case ResultType.LEAST_POPULAR_ORIGIN:
+                return group_and_sort ("subj_origin", where_sql, true, true);
+            case ResultType.MOST_RECENT_SUBJECT_INTERPRETATION:
+                return group_and_sort ("subj_interpretation", where_sql, false);
+            case ResultType.LEAST_RECENT_SUBJECT_INTERPRETATION:
+                return group_and_sort ("subj_interpretation", where_sql, true);
+            case ResultType.MOST_POPULAR_SUBJECT_INTERPRETATION:
+                return group_and_sort ("subj_interpretation", where_sql,
+                    false, false);
+            case ResultType.LEAST_POPULAR_SUBJECT_INTERPRETATION:
+                return group_and_sort ("subj_interpretation", where_sql,
+                    true, true);
+            case ResultType.MOST_RECENT_MIMETYPE:
+                return group_and_sort ("subj_mimetype", where_sql, false);
+            case ResultType.LEAST_RECENT_MIMETYPE:
+                return group_and_sort ("subj_mimetype", where_sql, true);
+            case ResultType.MOST_POPULAR_MIMETYPE:
+                return group_and_sort ("subj_mimetype", where_sql,
+                    false, false);
+            case ResultType.LEAST_POPULAR_MIMETYPE:
+                return group_and_sort ("subj_mimetype", where_sql,
+                    true, true);
+            default:
+                string error_message = "Invalid ResultType.";
+                warning (error_message);
+                throw new EngineError.INVALID_ARGUMENT (error_message);
+        }
+    }
+
     // Used by find_event_ids
     private string group_and_sort (string field, string where_sql,
         bool time_asc=false, bool? count_asc=null,

=== modified file 'src/engine.vala'
--- src/engine.vala	2012-03-14 14:26:11 +0000
+++ src/engine.vala	2012-03-19 17:30:32 +0000
@@ -277,6 +277,46 @@
         return event.id;
     }
 
+    public uint32[] delete_events_in_time_range (TimeRange range,
+        GenericArray<Event> event_templates,
+        uint storage_state, uint max_events, uint result_type,
+        BusName? sender=null) throws EngineError
+    {
+        uint32[] event_ids = find_event_ids (range, event_templates,
+            storage_state, max_events, result_type);
+
+        WhereClause where = get_where_clause_for_query (range, event_templates,
+            storage_state, sender);
+
+        string sql = "DELETE FROM event_view ";
+        string where_sql = "";
+        if (!where.is_empty ())
+        {
+            where_sql = "WHERE " + where.get_sql_conditions ();
+        }
+
+        sql += group_clause (result_type, where_sql);
+
+        if (max_events > 0)
+            sql += " LIMIT %u".printf(max_events);
+
+        int rc;
+        Sqlite.Statement stmt;
+
+        rc = db.prepare_v2 (sql, -1, out stmt);
+        database.assert_query_success(rc, "SQL error");
+
+        var arguments = where.get_bind_arguments ();
+        for (int i = 0; i < arguments.length; ++i)
+            stmt.bind_text (i + 1, arguments[i]);
+
+#if EXPLAIN_QUERIES
+        database.explain_query (stmt);
+#endif
+
+        return event_ids;
+    }
+
     public TimeRange? delete_events (uint32[] event_ids, BusName? sender)
         throws EngineError
         requires (event_ids.length > 0)

=== modified file 'src/remote.vala'
--- src/remote.vala	2012-02-10 17:03:50 +0000
+++ src/remote.vala	2012-03-19 17:30:32 +0000
@@ -38,6 +38,13 @@
             BusName sender
         ) throws Error;
 
+        public abstract uint32[] delete_events_in_time_range (
+            [DBus (signature = "(xx)")] Variant time_range,
+            [DBus (signature = "a(asaasay)")] Variant event_templates,
+            uint storage_state, uint num_events, uint result_type,
+            BusName sender
+        ) throws Error;
+
         public abstract uint32[] find_event_ids (
             [DBus (signature = "(xx)")] Variant time_range,
             [DBus (signature = "a(asaasay)")] Variant event_templates,

=== modified file 'src/zeitgeist-daemon.vala'
--- src/zeitgeist-daemon.vala	2012-03-01 14:47:30 +0000
+++ src/zeitgeist-daemon.vala	2012-03-19 17:30:32 +0000
@@ -205,6 +205,17 @@
             return event_ids;
         }
 
+        public uint32[] delete_events_in_time_range (Variant range,
+                Variant event_templates,
+                uint storage_state, uint num_events, uint result_type,
+                BusName sender) throws Error
+        {
+            uint32[] event_ids = engine.delete_events_in_time_range (new TimeRange.from_variant(range),
+                Events.from_variant (event_templates),
+                storage_state, num_events, result_type, sender);
+            return event_ids;
+        }
+
         public Variant delete_events (uint32[] event_ids, BusName sender)
             throws Error
         {