Merge lp:~rainct/zeitgeist/fts-sorting into lp:~zeitgeist/zeitgeist/bluebird

[Siegfried Gevatter]

[lp:~rainct/zeitgeist/fts-sorting]

433. By Seif Lotfy on 2012-03-17

Merged Trever's fixes for bug 953041

434. By Seif Lotfy on 2012-03-17

DS-Registry: fix vala 0.16 incompatiblity

435. By Seif Lotfy on 2012-03-19

Storage Monitor Extensions: fix bug lp:950983

436. By Seif Lotfy on 2012-03-19

Storage Monitor Extension: refactor the INSERT OR REPLACE statement

[Michal Hruby (mhr3)]

Looks fine.

review: Approve

[Michal Hruby (mhr3)]

But perhaps the is_asc method should be a (static?) method of ResultType.

[lp:~rainct/zeitgeist/fts-sorting]

437. By Siegfried Gevatter on 2012-03-19

Fix typo in table-lookup comment.

438. By Siegfried Gevatter on 2012-03-19

Refactor ResultType sorting and fix FTS++'s Search() with LeastRecent*

[Michal Hruby (mhr3)]

Yey!

review: Approve

=== modified file 'extensions/fts++/indexer.cpp'
--- extensions/fts++/indexer.cpp	2012-03-16 20:03:05 +0000
+++ extensions/fts++/indexer.cpp	2012-03-19 20:01:23 +0000
@@ -778,7 +778,9 @@
     }
     else
     {
-      enquire->set_sort_by_value (VALUE_TIMESTAMP, true);
+      bool reversed_sort = not
+          zeitgeist_result_type_is_sort_order_asc (result_type);
+      enquire->set_sort_by_value (VALUE_TIMESTAMP, reversed_sort);
     }
 
     if (result_type == ZEITGEIST_RESULT_TYPE_MOST_RECENT_SUBJECTS ||
@@ -786,7 +788,20 @@
         result_type == ZEITGEIST_RESULT_TYPE_MOST_POPULAR_SUBJECTS ||
         result_type == ZEITGEIST_RESULT_TYPE_LEAST_POPULAR_SUBJECTS)
     {
-        enquire->set_collapse_key (VALUE_URI_HASH);
+      enquire->set_collapse_key (VALUE_URI_HASH);
+    }
+    else if (result_type == ZEITGEIST_RESULT_TYPE_MOST_RECENT_ORIGIN ||
+        result_type == ZEITGEIST_RESULT_TYPE_LEAST_RECENT_ORIGIN ||
+        result_type == ZEITGEIST_RESULT_TYPE_MOST_POPULAR_ORIGIN ||
+        result_type == ZEITGEIST_RESULT_TYPE_LEAST_POPULAR_ORIGIN)
+    {
+      // FIXME: not really correct but close :)
+      enquire->set_collapse_key (VALUE_URI_HASH);
+    }
+    else if (result_type == ZEITGEIST_RESULT_TYPE_MOST_RECENT_EVENTS ||
+        result_type == ZEITGEIST_RESULT_TYPE_LEAST_RECENT_EVENTS)
+    {
+      enquire->set_collapse_key (VALUE_EVENT_ID);
     }
 
     Xapian::Query q(query_parser->parse_query (query_string, QUERY_PARSER_FLAGS));
@@ -1096,12 +1111,8 @@
       return NULL;
     }
 
-    bool reversed_sort =
-      result_type == ZEITGEIST_RESULT_TYPE_MOST_RECENT_EVENTS ||
-      result_type == ZEITGEIST_RESULT_TYPE_MOST_RECENT_SUBJECTS ||
-      result_type == ZEITGEIST_RESULT_TYPE_MOST_POPULAR_SUBJECTS ||
-      result_type == ZEITGEIST_RESULT_TYPE_MOST_RECENT_ORIGIN ||
-      result_type == ZEITGEIST_RESULT_TYPE_MOST_POPULAR_ORIGIN;
+    bool reversed_sort = not
+        zeitgeist_result_type_is_sort_order_asc (result_type);
 
     if (result_type == RELEVANCY_RESULT_TYPE)
     {
 
=== modified file 'src/datamodel.vala'
--- src/datamodel.vala	2012-02-18 21:33:57 +0000
+++ src/datamodel.vala	2012-03-19 20:01:23 +0000
@@ -230,10 +230,60 @@
                                                    // different origin ordered
                                                    // by the popularity of the
                                                    // origins
-        LEAST_POPULAR_EVENT_ORIGIN           = 30, //   The last event of each
+        LEAST_POPULAR_EVENT_ORIGIN           = 30; //   The last event of each
                                                    // different origin, ordered
                                                    // ascendingly by the
                                                    // popularity of the origin
+
+        /*
+         * Returns true if the results for the given result_type will be sorted
+         * ascendantly by date, false if they'll be sorted descendingly.
+         **/
+        public static bool is_sort_order_asc (ResultType result_type)
+        {
+            switch (result_type)
+            {
+                // FIXME: Why are LEAST_POPULAR_* using ASC?
+                case ResultType.LEAST_RECENT_EVENTS:
+                case ResultType.LEAST_RECENT_EVENT_ORIGIN:
+                case ResultType.LEAST_POPULAR_EVENT_ORIGIN:
+                case ResultType.LEAST_RECENT_SUBJECTS:
+                case ResultType.LEAST_POPULAR_SUBJECTS:
+                case ResultType.LEAST_RECENT_CURRENT_URI:
+                case ResultType.LEAST_POPULAR_CURRENT_URI:
+                case ResultType.LEAST_RECENT_ACTOR:
+                case ResultType.LEAST_POPULAR_ACTOR:
+                case ResultType.OLDEST_ACTOR:
+                case ResultType.LEAST_RECENT_ORIGIN:
+                case ResultType.LEAST_POPULAR_ORIGIN:
+                case ResultType.LEAST_RECENT_SUBJECT_INTERPRETATION:
+                case ResultType.LEAST_POPULAR_SUBJECT_INTERPRETATION:
+                case ResultType.LEAST_RECENT_MIMETYPE:
+                case ResultType.LEAST_POPULAR_MIMETYPE:
+                    return true;
+
+                case ResultType.MOST_RECENT_EVENTS:
+                case ResultType.MOST_RECENT_EVENT_ORIGIN:
+                case ResultType.MOST_POPULAR_EVENT_ORIGIN:
+                case ResultType.MOST_RECENT_SUBJECTS:
+                case ResultType.MOST_POPULAR_SUBJECTS:
+                case ResultType.MOST_RECENT_CURRENT_URI:
+                case ResultType.MOST_POPULAR_CURRENT_URI:
+                case ResultType.MOST_RECENT_ACTOR:
+                case ResultType.MOST_POPULAR_ACTOR:
+                case ResultType.MOST_RECENT_ORIGIN:
+                case ResultType.MOST_POPULAR_ORIGIN:
+                case ResultType.MOST_RECENT_SUBJECT_INTERPRETATION:
+                case ResultType.MOST_POPULAR_SUBJECT_INTERPRETATION:
+                case ResultType.MOST_RECENT_MIMETYPE:
+                case ResultType.MOST_POPULAR_MIMETYPE:
+                    return false;
+
+                default:
+                    warning ("Unrecognized ResultType: %u", (uint) result_type);
+                    return true;
+            }
+        }
     }
 
     /*
 
=== modified file 'src/db-reader.vala'
--- src/db-reader.vala	2012-03-17 14:47:09 +0000
+++ src/db-reader.vala	2012-03-19 20:01:23 +0000
@@ -173,109 +173,107 @@
         switch (result_type)
         {
             case ResultType.MOST_RECENT_EVENTS:
-                sql += where_sql + " ORDER BY timestamp DESC";
+                sql += where_sql + " ORDER BY ";
                 break;
             case ResultType.LEAST_RECENT_EVENTS:
-                sql += where_sql + " ORDER BY timestamp ASC";
+                sql += where_sql + " ORDER BY ";
                 break;
             case ResultType.MOST_RECENT_EVENT_ORIGIN:
-                sql += group_and_sort ("origin", where_sql, false);
+                sql += group_and_sort ("origin", where_sql);
                 break;
             case ResultType.LEAST_RECENT_EVENT_ORIGIN:
-                sql += group_and_sort ("origin", where_sql, true);
+                sql += group_and_sort ("origin", where_sql);
                 break;
             case ResultType.MOST_POPULAR_EVENT_ORIGIN:
-                sql += group_and_sort ("origin", where_sql, false, false);
+                sql += group_and_sort ("origin", where_sql, false);
                 break;
             case ResultType.LEAST_POPULAR_EVENT_ORIGIN:
-                sql += group_and_sort ("origin", where_sql, true, true);
+                sql += group_and_sort ("origin", where_sql, true);
                 break;
             case ResultType.MOST_RECENT_SUBJECTS:
-                sql += group_and_sort ("subj_id", where_sql, false);
+                sql += group_and_sort ("subj_id", where_sql);
                 break;
             case ResultType.LEAST_RECENT_SUBJECTS:
-                sql += group_and_sort ("subj_id", where_sql, true);
+                sql += group_and_sort ("subj_id", where_sql);
                 break;
             case ResultType.MOST_POPULAR_SUBJECTS:
-                sql += group_and_sort ("subj_id", where_sql, false, false);
+                sql += group_and_sort ("subj_id", where_sql, false);
                 break;
             case ResultType.LEAST_POPULAR_SUBJECTS:
-                sql += group_and_sort ("subj_id", where_sql, true, true);
+                sql += group_and_sort ("subj_id", where_sql, true);
                 break;
             case ResultType.MOST_RECENT_CURRENT_URI:
-                sql += group_and_sort ("subj_id_current", where_sql, false);
+                sql += group_and_sort ("subj_id_current", where_sql);
                 break;
             case ResultType.LEAST_RECENT_CURRENT_URI:
-                sql += group_and_sort ("subj_id_current", where_sql, true);
+                sql += group_and_sort ("subj_id_current", where_sql);
                 break;
             case ResultType.MOST_POPULAR_CURRENT_URI:
-                sql += group_and_sort ("subj_id_current", where_sql,
-                    false, false);
+                sql += group_and_sort ("subj_id_current", where_sql, false);
                 break;
             case ResultType.LEAST_POPULAR_CURRENT_URI:
-                sql += group_and_sort ("subj_id_current", where_sql,
-                    true, true);
+                sql += group_and_sort ("subj_id_current", where_sql, true);
                 break;
             case ResultType.MOST_RECENT_ACTOR:
-                sql += group_and_sort ("actor", where_sql, false);
+                sql += group_and_sort ("actor", where_sql);
                 break;
             case ResultType.LEAST_RECENT_ACTOR:
-                sql += group_and_sort ("actor", where_sql, true);
+                sql += group_and_sort ("actor", where_sql);
                 break;
             case ResultType.MOST_POPULAR_ACTOR:
-                sql += group_and_sort ("actor", where_sql, false, false);
+                sql += group_and_sort ("actor", where_sql, false);
                 break;
             case ResultType.LEAST_POPULAR_ACTOR:
-                sql += group_and_sort ("actor", where_sql, true, true);
+                sql += group_and_sort ("actor", where_sql, true);
                 break;
             case ResultType.OLDEST_ACTOR:
-                sql += group_and_sort ("actor", where_sql, true, null, "min");
+                sql += group_and_sort ("actor", where_sql, null, "min");
                 break;
             case ResultType.MOST_RECENT_ORIGIN:
-                sql += group_and_sort ("subj_origin", where_sql, false);
+                sql += group_and_sort ("subj_origin", where_sql);
                 break;
             case ResultType.LEAST_RECENT_ORIGIN:
-                sql += group_and_sort ("subj_origin", where_sql, true);
+                sql += group_and_sort ("subj_origin", where_sql);
                 break;
             case ResultType.MOST_POPULAR_ORIGIN:
-                sql += group_and_sort ("subj_origin", where_sql, false, false);
+                sql += group_and_sort ("subj_origin", where_sql, false);
                 break;
             case ResultType.LEAST_POPULAR_ORIGIN:
-                sql += group_and_sort ("subj_origin", where_sql, true, true);
+                sql += group_and_sort ("subj_origin", where_sql, true);
                 break;
             case ResultType.MOST_RECENT_SUBJECT_INTERPRETATION:
-                sql += group_and_sort ("subj_interpretation", where_sql, false);
+                sql += group_and_sort ("subj_interpretation", where_sql);
                 break;
             case ResultType.LEAST_RECENT_SUBJECT_INTERPRETATION:
-                sql += group_and_sort ("subj_interpretation", where_sql, true);
+                sql += group_and_sort ("subj_interpretation", where_sql);
                 break;
             case ResultType.MOST_POPULAR_SUBJECT_INTERPRETATION:
-                sql += group_and_sort ("subj_interpretation", where_sql,
-                    false, false);
+                sql += group_and_sort ("subj_interpretation", where_sql, false);
                 break;
             case ResultType.LEAST_POPULAR_SUBJECT_INTERPRETATION:
-                sql += group_and_sort ("subj_interpretation", where_sql,
-                    true, true);
+                sql += group_and_sort ("subj_interpretation", where_sql, true);
                 break;
             case ResultType.MOST_RECENT_MIMETYPE:
-                sql += group_and_sort ("subj_mimetype", where_sql, false);
+                sql += group_and_sort ("subj_mimetype", where_sql);
                 break;
             case ResultType.LEAST_RECENT_MIMETYPE:
-                sql += group_and_sort ("subj_mimetype", where_sql, true);
+                sql += group_and_sort ("subj_mimetype", where_sql);
                 break;
             case ResultType.MOST_POPULAR_MIMETYPE:
-                sql += group_and_sort ("subj_mimetype", where_sql,
-                    false, false);
+                sql += group_and_sort ("subj_mimetype", where_sql, false);
                 break;
             case ResultType.LEAST_POPULAR_MIMETYPE:
-                sql += group_and_sort ("subj_mimetype", where_sql,
-                    true, true);
+                sql += group_and_sort ("subj_mimetype", where_sql, true);
                 break;
             default:
                 string error_message = "Invalid ResultType.";
                 warning (error_message);
                 throw new EngineError.INVALID_ARGUMENT (error_message);
         }
+        
+        // complete the sort rule
+        bool time_asc = ResultType.is_sort_order_asc ((ResultType) result_type);
+        sql += " timestamp %s".printf ((time_asc) ? "ASC" : "DESC");
 
         int rc;
         Sqlite.Statement stmt;
@@ -578,10 +576,8 @@
 
     // Used by find_event_ids
     private string group_and_sort (string field, string where_sql,
-        bool time_asc=false, bool? count_asc=null,
-        string aggregation_type="max")
+        bool? count_asc=null, string aggregation_type="max")
     {
-        string time_sorting = (time_asc) ? "ASC" : "DESC";
         string aggregation_sql = "";
         string order_sql = "";
 
@@ -599,7 +595,7 @@
                 FROM event_view %s
                 GROUP BY %s)
             GROUP BY %s
-            ORDER BY %s timestamp %s
+            ORDER BY %s 
             """.printf (
                 field,
                 aggregation_type,
@@ -607,7 +603,7 @@
                 where_sql,
                 field,
                 field,
-                order_sql, time_sorting);
+                order_sql);
     }
 
     // Used by find_event_ids