[PATCH 1/3] lib: eagerly parse queries

Subject:[PATCH 1/3] lib: eagerly parse queries

Date:Sat, 25 Feb 2017 12:09:11 -0400

To:David Bremner ,notmuch@notmuchmail.org

Cc:

From:David Bremner


Rather than waiting for a call to count/search, parse the query string when the
notmuch_query_t is created. This is a small reduction in duplicated
code, and a potential efficiency improvement if many count/search
operations are called on the same query (although the latter sounds a
bit unusual). The main goal is to prepare the way for
non-destructive (or at least less destructive) exclude tag handling.

It does introduce a not-very-nice error path where running out of memory
is not easily distinguishable from a query syntax error.
---
 lib/query.cc | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/lib/query.cc b/lib/query.cc
index 4ccd8104..3cf63fc9 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -29,6 +29,7 @@ struct _notmuch_query {
     notmuch_sort_t sort;
     notmuch_string_list_t *exclude_terms;
     notmuch_exclude_t omit_excluded;
+    Xapian::Query xapian_query;
 };
 
 typedef struct _notmuch_mset_messages {
@@ -71,6 +72,13 @@ _debug_query (void)
     return (env && strcmp (env, "") != 0);
 }
 
+/* Explicit destructor call for placement new */
+static int
+_notmuch_query_destructor (notmuch_query_t *query) {
+    query->xapian_query.~Query();
+    return 0;
+}
+
 notmuch_query_t *
 notmuch_query_create (notmuch_database_t *notmuch,
 		      const char *query_string)
@@ -84,6 +92,10 @@ notmuch_query_create (notmuch_database_t *notmuch,
     if (unlikely (query == NULL))
 	return NULL;
 
+    new (&query->xapian_query) Xapian::Query ();
+
+    talloc_set_destructor (query, _notmuch_query_destructor);
+
     query->notmuch = notmuch;
 
     query->query_string = talloc_strdup (query, query_string);
@@ -94,6 +106,22 @@ notmuch_query_create (notmuch_database_t *notmuch,
 
     query->omit_excluded = NOTMUCH_EXCLUDE_TRUE;
 
+    try {
+	query->xapian_query =
+	    notmuch->query_parser->parse_query (query_string, NOTMUCH_QUERY_PARSER_FLAGS);
+    } catch (const Xapian::Error &error) {
+	_notmuch_database_log (notmuch,
+			       "A Xapian exception occured parsing query: %s\n",
+			       error.get_msg().c_str());
+	_notmuch_database_log_append (notmuch,
+			       "Query string was: %s\n",
+			       query->query_string);
+	query->notmuch->exception_reported = TRUE;
+
+	talloc_free (query);
+	query = NULL;
+    }
+
     return query;
 }
 
@@ -217,7 +245,7 @@ _notmuch_query_search_documents (notmuch_query_t *query,
 	Xapian::Query mail_query (talloc_asprintf (query, "%s%s",
 						   _find_prefix ("type"),
 						   type));
-	Xapian::Query string_query, final_query, exclude_query;
+	Xapian::Query final_query, exclude_query;
 	Xapian::MSet mset;
 	Xapian::MSetIterator iterator;
 
@@ -226,10 +254,8 @@ _notmuch_query_search_documents (notmuch_query_t *query,
 	{
 	    final_query = mail_query;
 	} else {
-	    string_query = notmuch->query_parser->
-		parse_query (query_string, NOTMUCH_QUERY_PARSER_FLAGS);
 	    final_query = Xapian::Query (Xapian::Query::OP_AND,
-					 mail_query, string_query);
+					 mail_query, query->xapian_query);
 	}
 	messages->base.excluded_doc_ids = NULL;
 
@@ -572,7 +598,7 @@ _notmuch_query_count_documents (notmuch_query_t *query, const char *type, unsign
 	Xapian::Query mail_query (talloc_asprintf (query, "%s%s",
 						   _find_prefix ("type"),
 						   type));
-	Xapian::Query string_query, final_query, exclude_query;
+	Xapian::Query final_query, exclude_query;
 	Xapian::MSet mset;
 
 	if (strcmp (query_string, "") == 0 ||
@@ -580,10 +606,8 @@ _notmuch_query_count_documents (notmuch_query_t *query, const char *type, unsign
 	{
 	    final_query = mail_query;
 	} else {
-	    string_query = notmuch->query_parser->
-		parse_query (query_string, NOTMUCH_QUERY_PARSER_FLAGS);
 	    final_query = Xapian::Query (Xapian::Query::OP_AND,
-					 mail_query, string_query);
+					 mail_query, query->xapian_query);
 	}
 
 	exclude_query = _notmuch_exclude_tags (query, final_query);
-- 
2.11.0


Thread: