Handle maybe UIDs separately from known good matches

In some edge cases, a maybe query can potentially break search results
because Dovecot's internal fallback querying does not have the necessary
contextual information to properly query (i.e. if the original text was
decoded from an attachment).

Fixed by handling the one "maybe" query flatcurve does (queries on
non-indexed headers) separately from the "main" full query that will
return definite results.

Fixes GitHub Issue #38

Author: slusarz

docs/data/events.data.js

@@ -37,7 +37,7 @@ export default {
         fields: {
           count: "The number of messages matched",
           mailbox: "The mailbox name",
-          maybe: "Are the results uncertain?",
+          maybe_uids: "The list of maybe UIDs returned by the query (these UIDs need to have their contents directly searched by Dovecot core)",
           query: "The query text sent to Xapian",
           uids: "The list of UIDs returned by the query"
         },

src/fts-backend-flatcurve-xapian.cpp

@@ -15,6 +15,7 @@
 #include "fts-flatcurve-config.h"
 extern "C" {
 #include "lib.h"
+#include "array.h"
 #include "file-create-locked.h"
 #include "hash.h"
 #include "mail-storage-private.h"
@@ -148,8 +149,13 @@ struct flatcurve_xapian {
 	bool closing:1;
 };
 
+struct flatcurve_fts_query_xapian_maybe {
+	Xapian::Query *query;
+};
+
 struct flatcurve_fts_query_xapian {
 	Xapian::Query *query;
+	ARRAY(struct flatcurve_fts_query_xapian_maybe) maybe_queries;
 };
 
 struct flatcurve_xapian_db_iter {
@@ -185,6 +191,7 @@ struct fts_flatcurve_xapian_query_iter {
 	Xapian::Enquire *enquire;
 	Xapian::MSetIterator i;
 	struct fts_flatcurve_xapian_query_result *result;
+	int curr_query;
 };
 
 static void
@@ -1465,6 +1472,8 @@ fts_flatcurve_build_query_arg_term(struct flatcurve_fts_query *query,
 				   const char *term)
 {
 	const char *hdr;
+	bool maybe = FALSE;
+	struct flatcurve_fts_query_xapian_maybe *mquery;
 	Xapian::Query::op op = Xapian::Query::OP_INVALID;
 	Xapian::Query *oldq, q;
 	struct flatcurve_fts_query_xapian *x = query->xapian;
@@ -1529,7 +1538,7 @@ fts_flatcurve_build_query_arg_term(struct flatcurve_fts_query *query,
 				 * appears in the general pool of header
 				 * terms for the message, not to a specific
 				 * header, so this is only a maybe match. */
-				query->maybe = TRUE;
+				maybe = TRUE;
 			}
 		} else {
 			hdr = t_str_lcase(arg->hdr_field_name);
@@ -1545,9 +1554,17 @@ fts_flatcurve_build_query_arg_term(struct flatcurve_fts_query *query,
 		q = Xapian::Query(Xapian::Query::OP_AND_NOT,
 				  Xapian::Query::MatchAll, q);
 
-	if (x->query == NULL)
+	if (maybe) {
+		/* Maybe searches are not added to the "master search" query; they
+		 * will be run independently. Matches will be placed in the maybe
+		 * results array. */
+		if (!array_is_created(&x->maybe_queries))
+			p_array_init(&x->maybe_queries, query->pool, 4);
+		mquery = array_append_space(&x->maybe_queries);
+		mquery->query = new Xapian::Query(std_move(q));
+	} else if (x->query == NULL) {
 		x->query = new Xapian::Query(std_move(q));
-	else {
+	} else {
 		oldq = x->query;
 		x->query = new Xapian::Query(op, *(x->query), q);
 		delete(oldq);
@@ -1642,29 +1659,59 @@ fts_flatcurve_xapian_query_iter_init(struct flatcurve_fts_query *query)
 	struct fts_flatcurve_xapian_query_iter *iter;
 
 	iter = p_new(query->pool, struct fts_flatcurve_xapian_query_iter, 1);
+	/* -1: "Master" query
+	 * >= 0: Current index of maybe_queries */
+	iter->curr_query = -1;
 	iter->query = query;
 	iter->result = p_new(query->pool,
 			     struct fts_flatcurve_xapian_query_result, 1);
 
 	return iter;
 }
 
+static void
+fts_flatcurve_xapian_query_iter_clear_current(struct fts_flatcurve_xapian_query_iter *iter)
+{
+	iter->i.~MSetIterator();
+	delete(iter->enquire);
+	iter->enquire = NULL;
+}
+
 struct fts_flatcurve_xapian_query_result *
 fts_flatcurve_xapian_query_iter_next(struct fts_flatcurve_xapian_query_iter *iter)
 {
 	Xapian::MSet m;
+	const struct flatcurve_fts_query_xapian_maybe *mquery;
 	enum flatcurve_xapian_db_opts opts =
 		ENUM_EMPTY(flatcurve_xapian_db_opts);
+	Xapian::Query *q = NULL;
 
 	if (iter->enquire == NULL) {
-		if ((iter->query->xapian->query == NULL) ||
+		/* Master query. */
+		if (iter->curr_query == -1) {
+			if (iter->query->xapian->query == NULL)
+				++iter->curr_query;
+			else
+				q = iter->query->xapian->query;
+		}
+
+		/* Maybe queries. */
+		if ((iter->curr_query >= 0) &&
+			(array_is_created(&iter->query->xapian->maybe_queries)) &&
+			(array_count(&iter->query->xapian->maybe_queries) > iter->curr_query)) {
+			mquery = array_idx(&iter->query->xapian->maybe_queries,
+							   iter->curr_query);
+			q = mquery->query;
+		}
+
+		if ((q == NULL) ||
 		    ((iter->db = fts_flatcurve_xapian_read_db(iter->query->backend, opts)) == NULL))
 			return NULL;
 
 		iter->enquire = new Xapian::Enquire(*iter->db);
 		iter->enquire->set_docid_order(
 				Xapian::Enquire::DONT_CARE);
-		iter->enquire->set_query(*iter->query->xapian->query);
+		iter->enquire->set_query(*q);
 
 		try {
 			m = iter->enquire->get_mset(0,
@@ -1685,9 +1732,13 @@ fts_flatcurve_xapian_query_iter_next(struct fts_flatcurve_xapian_query_iter *ite
 		iter->i = m.begin();
 	}
 
-	if (iter->i == m.end())
-		return NULL;
+	if (iter->i == m.end()) {
+		fts_flatcurve_xapian_query_iter_clear_current(iter);
+		++iter->curr_query;
+		return fts_flatcurve_xapian_query_iter_next(iter);
+	}
 
+	iter->result->maybe = (iter->curr_query >= 0);
 	iter->result->score = iter->i.get_weight();
 	/* MSet docid can be an "interleaved" docid generated by
 	 * Xapian::Database when handling multiple DBs at once. Instead, we
@@ -1707,8 +1758,7 @@ fts_flatcurve_xapian_query_iter_deinit(struct fts_flatcurve_xapian_query_iter **
 	/* Need to explicitly call dtor, or else MSet doesn't release memory
 	 * allocated internally. */
 	*_iter = NULL;
-	iter->i.~MSetIterator();
-	delete(iter->enquire);
+	fts_flatcurve_xapian_query_iter_clear_current(iter);
 	p_free(iter->query->pool, iter->result);
 	p_free(iter->query->pool, iter);
 }
@@ -1723,7 +1773,10 @@ bool fts_flatcurve_xapian_run_query(struct flatcurve_fts_query *query,
 	if ((iter = fts_flatcurve_xapian_query_iter_init(query)) == NULL)
 		return FALSE;
 	while ((result = fts_flatcurve_xapian_query_iter_next(iter)) != NULL) {
-		seq_range_array_add(&r->uids, result->uid);
+		if (result->maybe)
+			seq_range_array_add(&r->maybe_uids, result->uid);
+		else
+			seq_range_array_add(&r->uids, result->uid);
 		score = array_append_space(&r->scores);
 		score->score = (float)result->score;
 		score->uid = result->uid;
@@ -1734,7 +1787,15 @@ bool fts_flatcurve_xapian_run_query(struct flatcurve_fts_query *query,
 
 void fts_flatcurve_xapian_destroy_query(struct flatcurve_fts_query *query)
 {
+	struct flatcurve_fts_query_xapian_maybe *mquery;
+
 	delete(query->xapian->query);
+	if (array_is_created(&query->xapian->maybe_queries)) {
+		array_foreach_modifiable(&query->xapian->maybe_queries, mquery) {
+			delete(mquery->query);
+		}
+		array_free(&query->xapian->maybe_queries);
+	}
 }
 
 const char *fts_flatcurve_xapian_library_version()

src/fts-backend-flatcurve-xapian.h

@@ -7,6 +7,8 @@
 struct fts_flatcurve_xapian_query_result {
 	double score;
 	uint32_t uid;
+
+	bool maybe:1;
 };
 
 struct fts_flatcurve_xapian_db_check {

src/fts-backend-flatcurve.c

@@ -548,10 +548,10 @@ fts_backend_flatcurve_lookup_multi(struct fts_backend *_backend,
 	ARRAY(struct fts_result) box_results;
 	struct flatcurve_fts_result *fresult;
 	unsigned int i;
+	const char *m_debug = "", *u_debug = "";
 	struct flatcurve_fts_query *query;
 	struct fts_result *r;
 	int ret = 0;
-	const char *u;
 
 	/* Create query */
 	query = fts_backend_flatcurve_create_query(backend, result->pool);
@@ -565,6 +565,7 @@ fts_backend_flatcurve_lookup_multi(struct fts_backend *_backend,
 		r->box = boxes[i];
 
 		fresult = p_new(result->pool, struct flatcurve_fts_result, 1);
+		p_array_init(&fresult->maybe_uids, result->pool, 32);
 		p_array_init(&fresult->scores, result->pool, 32);
 		p_array_init(&fresult->uids, result->pool, 32);
 
@@ -575,29 +576,31 @@ fts_backend_flatcurve_lookup_multi(struct fts_backend *_backend,
 			break;
 		}
 
-		if ((query->maybe) ||
-		    ((flags & FTS_LOOKUP_FLAG_NO_AUTO_FUZZY) != 0))
-			r->maybe_uids = fresult->uids;
-		else
-			r->definite_uids = fresult->uids;
+		r->definite_uids = fresult->uids;
+		r->maybe_uids = fresult->maybe_uids;
 		r->scores = fresult->scores;
 
 		/* This was an empty query - skip output of debug info. */
 		if (!str_len(query->qtext))
 			continue;
 
-		u = str_c(fts_backend_flatcurve_seq_range_string(&fresult->uids,
-								 query->pool));
+		if (array_not_empty(&fresult->maybe_uids))
+			m_debug = str_c(fts_backend_flatcurve_seq_range_string(
+								&fresult->maybe_uids, query->pool));
+		if (array_not_empty(&fresult->uids))
+			u_debug = str_c(fts_backend_flatcurve_seq_range_string(
+								&fresult->uids, query->pool));
+
 		e_debug(event_create_passthrough(backend->event)->
 			set_name("fts_flatcurve_query")->
 			add_int("count", array_count(&fresult->uids))->
 			add_str("mailbox", r->box->vname)->
-			add_str("maybe", query->maybe ? "yes" : "no")->
+			add_str("maybe_uids", m_debug)->
 			add_str("query", str_c(query->qtext))->
-			add_str("uids", u)->event(), "Query (%s) "
-			"%smatches=%d uids=%s", str_c(query->qtext),
-			query->maybe ? "maybe_" : "",
-			array_count(&fresult->uids), u);
+			add_str("uids", u_debug)->event(), "Query (%s) "
+			"matches=%d uids=%s maybe_matches=%d maybe_uids=%s",
+			str_c(query->qtext), array_count(&fresult->uids), u_debug,
+			array_count(&fresult->maybe_uids), m_debug);
 	}
 
 	if (ret == 0) {

src/fts-backend-flatcurve.h

@@ -50,11 +50,11 @@ struct flatcurve_fts_query {
 	pool_t pool;
 
 	bool match_all:1;
-	bool maybe:1;
 };
 
 struct flatcurve_fts_result {
 	ARRAY_TYPE(fts_score_map) scores;
+	ARRAY_TYPE(seq_range) maybe_uids;
 	ARRAY_TYPE(seq_range) uids;
 };