[PATCH 2/3] lib: Add support for automatically excluding tags from queries
Austin Clements
amdragon at MIT.EDU
Wed Jan 11 10:48:38 PST 2012
Quoth Jani Nikula on Jan 11 at 10:11 am:
> On Wed, 11 Jan 2012 00:02:52 -0500, Austin Clements <amdragon at MIT.EDU> wrote:
> > This is useful for tags like "deleted" and "spam" that people
> > generally want to exclude from query results. These exclusions will
> > be overridden if a tag is explicitly mentioned in a query.
> > ---
> > lib/notmuch.h | 6 ++++++
> > lib/query.cc | 33 +++++++++++++++++++++++++++++++++
> > 2 files changed, 39 insertions(+), 0 deletions(-)
> >
> > diff --git a/lib/notmuch.h b/lib/notmuch.h
> > index 9f23a10..0a3ae2b 100644
> > --- a/lib/notmuch.h
> > +++ b/lib/notmuch.h
> > @@ -457,6 +457,12 @@ notmuch_query_set_sort (notmuch_query_t *query, notmuch_sort_t sort);
> > notmuch_sort_t
> > notmuch_query_get_sort (notmuch_query_t *query);
> >
> > +/* Add a tag that will be excluded by default from the query results.
> > + * This exclusion will be overridden if this tag appears explicitly in
> > + * the query. */
> > +void
> > +notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag);
> > +
> > /* Execute a query for threads, returning a notmuch_threads_t object
> > * which can be used to iterate over the results. The returned threads
> > * object is owned by the query and as such, will only be valid until
> > diff --git a/lib/query.cc b/lib/query.cc
> > index b6c0f12..716db1c 100644
> > --- a/lib/query.cc
> > +++ b/lib/query.cc
> > @@ -27,6 +27,7 @@ struct _notmuch_query {
> > notmuch_database_t *notmuch;
> > const char *query_string;
> > notmuch_sort_t sort;
> > + notmuch_string_list_t *exclude_terms;
> > };
> >
> > typedef struct _notmuch_mset_messages {
> > @@ -76,6 +77,8 @@ notmuch_query_create (notmuch_database_t *notmuch,
> >
> > query->sort = NOTMUCH_SORT_NEWEST_FIRST;
> >
> > + query->exclude_terms = _notmuch_string_list_create (query);
> > +
> > return query;
> > }
> >
> > @@ -97,6 +100,13 @@ notmuch_query_get_sort (notmuch_query_t *query)
> > return query->sort;
> > }
> >
> > +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_string_list_append (query->exclude_terms, term);
> > +}
> > +
> > /* We end up having to call the destructors explicitly because we had
> > * to use "placement new" in order to initialize C++ objects within a
> > * block that we allocated with talloc. So C++ is making talloc
> > @@ -112,6 +122,25 @@ _notmuch_messages_destructor (notmuch_mset_messages_t *messages)
> > return 0;
> > }
> >
>
> I'd like to have a comment here, or inline in the code, explaining the
> following function a little bit.
Done.
> > +static Xapian::Query
> > +_notmuch_exclude_tags (notmuch_query_t *query, Xapian::Query xquery)
> > +{
> > + Xapian::TermIterator end = xquery.get_terms_end ();
> > +
> > + for (notmuch_string_node_t *term = query->exclude_terms->head; term;
> > + term = term->next) {
> > + Xapian::TermIterator it = xquery.get_terms_begin ();
> > + for (; it != end; it++) {
> > + if (*it == term->string)
>
> [This is a double reminder to me why I'm not that enthusiastic about
> operator overloading in C++.]
Actually, that's a good point. I had originally done this to prevent
std::string from performing any internal allocation or copying (which
calling c_str could do), but since it took me some digging to figure
out if I had *actually* avoided this (turns out there's an overloaded
string==char*, so I had), I've switched to using string.compare, which
I think makes the behavior and performance more obvious.
> > + break;
> > + }
> > + if (it == end)
> > + xquery = Xapian::Query (Xapian::Query::OP_AND_NOT,
> > + xquery, Xapian::Query (term->string));
>
> I briefly dug into Xapian documentation and source code, and became none
> the wiser whether this copies the queries passed to it or not, i.e. does
> this leak memory or not. I just presume you know what you're doing. ;)
Since my code isn't doing any memory allocation, it can't leak memory.
It could do a lot of copying, but it turns out that isn't a problem
either because Xapian objects are internally reference-counted
handles. So, in other words, if you write the obvious thing, it will
do the right thing, which is totally contrary to what C++ has
conditioned us to expect. ]:--8)
> I think the function fails if someone is stupid enough to exclude the
> same tag twice. I'm not sure if you should care. If you think so, you
> could just check double add in notmuch_query_add_tag_exclude().
It handles this correctly for two reasons. Suppose tag x is excluded
twice. At worst, if tag:x doesn't appear in the query, the returned
query will get two AND NOT tag:x's, but that doesn't affect the
results. It turns out it doesn't even get doubled up for a slightly
subtle reason: when the loop reaches the second x in the exclude list,
it will iterate over the new query, which already has the AND NOT
tag:x in it, so it will see the tag:x as if it were in the original
query and not further modify the query.
However, this did point out a bug. I was using the end iterator from
the original query even when I started iterating over the modified
query.
I'll send out an updated series after someone looks at the third
patch.
> Otherwise, looks good.
>
> > + }
> > + return xquery;
> > +}
> > +
> > notmuch_messages_t *
> > notmuch_query_search_messages (notmuch_query_t *query)
> > {
> > @@ -157,6 +186,8 @@ notmuch_query_search_messages (notmuch_query_t *query)
> > mail_query, string_query);
> > }
> >
> > + final_query = _notmuch_exclude_tags (query, final_query);
> > +
> > enquire.set_weighting_scheme (Xapian::BoolWeight());
> >
> > switch (query->sort) {
> > @@ -436,6 +467,8 @@ notmuch_query_count_messages (notmuch_query_t *query)
> > mail_query, string_query);
> > }
> >
> > + final_query = _notmuch_exclude_tags (query, final_query);
> > +
> > enquire.set_weighting_scheme(Xapian::BoolWeight());
> > enquire.set_docid_order(Xapian::Enquire::ASCENDING);
> >
>
More information about the notmuch
mailing list