Merge lp:~zeitgeist/zeitgeist/bluebird-no-distinct into lp:~zeitgeist/zeitgeist/bluebird

[Siegfried Gevatter]

[Siegfried Gevatter (rainct)]

From #sqlite:
-----------------------------------------------
<RainCT> Hi. I've got a question: do .step() calls (internally) just look for the next row, or does the first .step() call already process everything?
<RainCT> Particularly, I'm wondering if removing a LIMIT from a query and instead aborting the while(... stmt.step() ...) once I've got enough will have any bad side effect
<#sqlite_guy> RainCT: technically, each step is actually executing code on the internal VM
<#sqlite_guy> I would say that if it makes sense to put in a LIMIT, then do so
<RainCT> #sqlite_guy: the problem is that the limit depends on another condition (a DISTINCT), but having SQLite check that makes the query 100x slower
<#sqlite_guy> how messy is the distinct?
<#sqlite_guy> could you throw an appropriate index on?
<RainCT> and I can really easily check that condition in code and count the number of things I got myself
<#sqlite_guy> honestly, you're up in the realm of personal taste/whatever works
<RainCT> #sqlite_guy: it's basically an integer ID which may be the same for a few consecutive rows
<RainCT> #sqlite_guy: Okay, thanks :). On benchmarks it seems to work fine, but I just wanted to make sure I'm not missing something.
<#sqlite_guy> nah
<#sqlite_guy> you're not missing anything
<#sqlite_guy> just make sure to hit sqlite3_reset when you're done with it
<#sqlite_guy> or finalize, obviously
-----------------------------------------------

[Michal Hruby (mhr3)]

33 + event_ids += event_id;

We could be more clever here, this will cause 3-6 reallocations in common cases, perhaps the array should be preallocated to `int.min(max_events, 128)`? (for non-zero max_events).

[lp:~zeitgeist/zeitgeist/bluebird-no-distinct]

355. By Siegfried Gevatter on 2011-12-31

benchmark-tools: fix symlink and usage example.

356. By Siegfried Gevatter on 2011-12-31

Using DISTINCT in FindEventIds has a huge performance penalty, so we
do the filtering and counting ourselves instead.

[Michal Hruby (mhr3)]

As the query plans show, without the DISTINCT sql doesn't need to build one temp b-tree, so it's slightly faster.

review: Approve

=== modified file 'src/engine.vala'
--- src/engine.vala	2011-12-29 13:59:16 +0000
+++ src/engine.vala	2011-12-31 14:59:23 +0000
@@ -207,7 +207,7 @@
         //if (!where.may_have_results ())
         //    return new uint32[0];
 
-        string sql = "SELECT DISTINCT id FROM event_view ";
+        string sql = "SELECT id FROM event_view ";
         string where_sql = "";
         if (!where.is_empty ())
         {
@@ -321,9 +321,6 @@
                 throw new EngineError.INVALID_ARGUMENT (error_message);
         }
 
-        if (max_events > 0)
-            sql += " LIMIT %u".printf (max_events);
-
         int rc;
         Sqlite.Statement stmt;
 
@@ -338,11 +335,15 @@
 
         while ((rc = stmt.step()) == Sqlite.ROW)
         {
-            var id = (uint32) uint64.parse(
+            var event_id = (uint32) uint64.parse(
                 stmt.column_text (EventViewRows.ID));
-            event_ids += id;
+            // Events are supposed to be contiguous in the database
+            if (event_ids.length == 0 || event_ids[event_ids.length-1] != event_id) {
+                event_ids += event_id;
+                if (event_ids.length == max_events) break;
+            }
         }
-        if (rc != Sqlite.DONE)
+        if (rc != Sqlite.DONE && rc != Sqlite.ROW)
         {
             string error_message = "Error in find_event_ids: %d, %s".printf (
                 rc, db.errmsg ());