From: David Bremner <david@tethera.net>
To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org
Subject: [PATCH 3/3] lib: query make exclude handling non-destructive
Date: Sat, 25 Feb 2017 12:09:13 -0400 [thread overview]
Message-ID: <20170225160913.22844-4-david@tethera.net> (raw)
In-Reply-To: <20170225160913.22844-1-david@tethera.net>
We filter added exclude at add time, rather than modifying the query by
count search. As noted in the comments, there are several ignored
conditions here.
---
lib/query.cc | 55 ++++++++++++++++++++++++++++++++++++------------------
test/T060-count.sh | 1 -
2 files changed, 37 insertions(+), 19 deletions(-)
diff --git a/lib/query.cc b/lib/query.cc
index bab8a60f..2c6a4ba6 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -31,6 +31,7 @@ struct _notmuch_query {
notmuch_exclude_t omit_excluded;
notmuch_bool_t parsed;
Xapian::Query xapian_query;
+ std::set<std::string> terms;
};
typedef struct _notmuch_mset_messages {
@@ -77,6 +78,7 @@ _debug_query (void)
static int
_notmuch_query_destructor (notmuch_query_t *query) {
query->xapian_query.~Query();
+ query->terms.~set<std::string>();
return 0;
}
@@ -94,6 +96,7 @@ notmuch_query_create (notmuch_database_t *notmuch,
return NULL;
new (&query->xapian_query) Xapian::Query ();
+ new (&query->terms) std::set<std::string> ();
query->parsed = FALSE;
talloc_set_destructor (query, _notmuch_query_destructor);
@@ -122,6 +125,15 @@ _notmuch_query_ensure_parsed (notmuch_query_t *query)
query->notmuch->query_parser->
parse_query (query->query_string, NOTMUCH_QUERY_PARSER_FLAGS);
+ /* Xapian doesn't support skip_to on terms from a query since
+ * they are unordered, so cache a copy of all terms in
+ * something searchable.
+ */
+
+ for (Xapian::TermIterator t = query->xapian_query.get_terms_begin ();
+ t != query->xapian_query.get_terms_end (); ++t)
+ query->terms.insert (*t);
+
query->parsed = TRUE;
} catch (const Xapian::Error &error) {
@@ -166,7 +178,25 @@ notmuch_query_get_sort (const notmuch_query_t *query)
void
notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
{
- char *term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
+ notmuch_status_t status;
+ char *term;
+
+ status = _notmuch_query_ensure_parsed (query);
+ /* The following is not ideal error handling, but to avoid
+ * breaking the ABI, we can live with it for now. In particular at
+ * least in the notmuch CLI, any syntax error in the query is
+ * caught in a later call to _notmuch_query_ensure_parsed with a
+ * better error path.
+ *
+ * TODO: add status return to this function.
+ */
+ if (status)
+ return;
+
+ term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
+ if (query->terms.count(term) != 0)
+ return; /* XXX report ignoring exclude? */
+
_notmuch_string_list_append (query->exclude_terms, term);
}
@@ -186,28 +216,17 @@ _notmuch_messages_destructor (notmuch_mset_messages_t *messages)
}
/* Return a query that matches messages with the excluded tags
- * registered with query. Any tags that explicitly appear in xquery
- * will not be excluded, and will be removed from the list of exclude
- * tags. The caller of this function has to combine the returned
+ * registered with query. The caller of this function has to combine the returned
* query appropriately.*/
static Xapian::Query
-_notmuch_exclude_tags (notmuch_query_t *query, Xapian::Query xquery)
+_notmuch_exclude_tags (notmuch_query_t *query)
{
Xapian::Query exclude_query = Xapian::Query::MatchNothing;
for (notmuch_string_node_t *term = query->exclude_terms->head; term;
term = term->next) {
- Xapian::TermIterator it = xquery.get_terms_begin ();
- Xapian::TermIterator end = xquery.get_terms_end ();
- for (; it != end; it++) {
- if ((*it).compare (term->string) == 0)
- break;
- }
- if (it == end)
- exclude_query = Xapian::Query (Xapian::Query::OP_OR,
- exclude_query, Xapian::Query (term->string));
- else
- term->string = talloc_strdup (query, "");
+ exclude_query = Xapian::Query (Xapian::Query::OP_OR,
+ exclude_query, Xapian::Query (term->string));
}
return exclude_query;
}
@@ -278,7 +297,7 @@ _notmuch_query_search_documents (notmuch_query_t *query,
messages->base.excluded_doc_ids = NULL;
if ((query->omit_excluded != NOTMUCH_EXCLUDE_FALSE) && (query->exclude_terms)) {
- exclude_query = _notmuch_exclude_tags (query, final_query);
+ exclude_query = _notmuch_exclude_tags (query);
if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
query->omit_excluded == NOTMUCH_EXCLUDE_ALL)
@@ -633,7 +652,7 @@ _notmuch_query_count_documents (notmuch_query_t *query, const char *type, unsign
mail_query, query->xapian_query);
}
- exclude_query = _notmuch_exclude_tags (query, final_query);
+ exclude_query = _notmuch_exclude_tags (query);
final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
final_query, exclude_query);
diff --git a/test/T060-count.sh b/test/T060-count.sh
index d27e1bab..4751440e 100755
--- a/test/T060-count.sh
+++ b/test/T060-count.sh
@@ -127,7 +127,6 @@ test_expect_equal_file EXPECTED OUTPUT.clean
restore_database
test_begin_subtest "count library function is non-destructive"
-test_subtest_known_broken
cat<<EOF > EXPECTED
1: 52 messages
2: 52 messages
--
2.11.0
prev parent reply other threads:[~2017-02-25 16:09 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-18 15:08 v4 of nondestructive excludes patches David Bremner
2017-02-18 15:08 ` [PATCH 1/4] lib: eagerly parse queries David Bremner
2017-02-18 15:08 ` [PATCH 2/4] lib/query: make query parsing lazy again, keep centralized David Bremner
2017-02-18 15:08 ` [PATCH 3/4] lib: query make exclude handling non-destructive David Bremner
2017-02-18 15:08 ` [PATCH 4/4] lib: make notmuch_query_add_tag_exclude return a status value David Bremner
2017-03-22 11:54 ` David Bremner
2017-02-25 16:09 ` v5 of nondestructive includes patches David Bremner
2017-02-25 16:09 ` [PATCH 1/3] lib: eagerly parse queries David Bremner
2017-02-25 16:09 ` [PATCH 2/3] lib/query: make query parsing lazy again, keep centralized David Bremner
2017-02-25 16:09 ` David Bremner [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170225160913.22844-4-david@tethera.net \
--to=david@tethera.net \
--cc=notmuch@notmuchmail.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).