Merge lp:~tdfischer/zeitgeist/common-where into lp:~zeitgeist/zeitgeist/bluebird

[Trever Fischer]

Consolidates a lot of WHERE SQL generation into a common method for re-use by other components.

[Siegfried Gevatter (rainct)]

Hey,

Thanks for splitting this out :).

> public uint32[] find_event_ids_for_clause (WhereClause where,
> uint max_events, uint result_type,
> BusName? sender=null) throws EngineError

This method isn't using `sender' anywhere (nor should it), so drop that argument.

> public WhereClause get_where_clause_for_query

Same here.

review: Needs Fixing

[lp:~tdfischer/zeitgeist/common-where]

422. By Trever Fischer on 2012-03-13

Remove unused argument

[Trever Fischer (tdfischer)]

Updated branch with removed arguments.

[Siegfried Gevatter (rainct)]

review: Approve

=== modified file 'src/db-reader.vala'
--- src/db-reader.vala	2012-02-05 14:52:13 +0000
+++ src/db-reader.vala	2012-03-13 18:56:18 +0000
@@ -160,47 +160,9 @@
         return results;
     }
 
-    public uint32[] find_event_ids (TimeRange time_range,
-        GenericArray<Event> event_templates,
-        uint storage_state, uint max_events, uint result_type,
-        BusName? sender=null) throws EngineError
+    public uint32[] find_event_ids_for_clause (WhereClause where,
+        uint max_events, uint result_type) throws EngineError
     {
-
-        WhereClause where = new WhereClause (WhereClause.Type.AND);
-
-        /**
-         * We are using the unary operator here to tell SQLite to not use
-         * the index on the timestamp column at the first place. This is a
-         * "fix" for (LP: #672965) based on some benchmarks, which suggest
-         * a performance win, but we might not oversee all implications.
-         * (See http://www.sqlite.org/optoverview.html, section 6.0).
-         *    -- Markus Korn, 29/11/2010
-         */
-        if (time_range.start != 0)
-            where.add (("+timestamp >= %" + int64.FORMAT).printf(
-                time_range.start));
-        if (time_range.end != 0)
-            where.add (("+timestamp <= %" + int64.FORMAT).printf(
-                time_range.end));
-
-        if (storage_state == StorageState.AVAILABLE ||
-            storage_state == StorageState.NOT_AVAILABLE)
-        {
-            where.add ("(subj_storage_state=? OR subj_storage_state IS NULL)",
-                storage_state.to_string ());
-        }
-        else if (storage_state != StorageState.ANY)
-        {
-            throw new EngineError.INVALID_ARGUMENT(
-                "Unknown storage state '%u'".printf(storage_state));
-        }
-
-        WhereClause tpl_conditions = get_where_clause_from_event_templates (
-            event_templates);
-        where.extend (tpl_conditions);
-        //if (!where.may_have_results ())
-        //    return new uint32[0];
-
         string sql = "SELECT id FROM event_view ";
         string where_sql = "";
         if (!where.is_empty ())
@@ -352,6 +314,20 @@
         return event_ids;
     }
 
+    public uint32[] find_event_ids (TimeRange time_range,
+        GenericArray<Event> event_templates,
+        uint storage_state, uint max_events, uint result_type,
+        BusName? sender=null) throws EngineError
+    {
+        WhereClause where = get_where_clause_for_query (time_range,
+            event_templates, storage_state);
+
+        //if (!where.may_have_results ())
+        //    return new uint32[0];
+
+        return find_event_ids_for_clause (where, max_events, result_type);
+    }
+
     public GenericArray<Event?> find_events (TimeRange time_range,
         GenericArray<Event> event_templates,
         uint storage_state, uint max_events, uint result_type,
@@ -361,6 +337,45 @@
             storage_state, max_events, result_type));
     }
 
+    public WhereClause get_where_clause_for_query (TimeRange time_range,
+        GenericArray<Event> event_templates, uint storage_state) throws EngineError
+    {
+        WhereClause where = new WhereClause (WhereClause.Type.AND);
+
+        /**
+         * We are using the unary operator here to tell SQLite to not use
+         * the index on the timestamp column at the first place. This is a
+         * "fix" for (LP: #672965) based on some benchmarks, which suggest
+         * a performance win, but we might not oversee all implications.
+         * (See http://www.sqlite.org/optoverview.html, section 6.0).
+         *    -- Markus Korn, 29/11/2010
+         */
+        if (time_range.start != 0)
+            where.add (("+timestamp >= %" + int64.FORMAT).printf(
+                time_range.start));
+        if (time_range.end != 0)
+            where.add (("+timestamp <= %" + int64.FORMAT).printf(
+                time_range.end));
+
+        if (storage_state == StorageState.AVAILABLE ||
+            storage_state == StorageState.NOT_AVAILABLE)
+        {
+            where.add ("(subj_storage_state=? OR subj_storage_state IS NULL)",
+                storage_state.to_string ());
+        }
+        else if (storage_state != StorageState.ANY)
+        {
+            throw new EngineError.INVALID_ARGUMENT(
+                "Unknown storage state '%u'".printf(storage_state));
+        }
+
+        WhereClause tpl_conditions = get_where_clause_from_event_templates (
+            event_templates);
+        where.extend (tpl_conditions);
+
+        return where;
+    }
+
     private struct RelatedUri {
         public uint32 id;
         public int64 timestamp;
@@ -596,7 +611,7 @@
     }
 
     // Used by find_event_ids
-    protected WhereClause get_where_clause_from_event_templates (
+    public WhereClause get_where_clause_from_event_templates (
         GenericArray<Event> templates) throws EngineError
     {
         WhereClause where = new WhereClause (WhereClause.Type.OR);