Ticket #157: boolgroups.4.patch

queryparser/queryparser.lemony

@@ -25 +25 @@
 #include "queryparser_internal.h"
 #include <xapian/unicode.h>
 #include "utils.h"
+#include "autoptr.h"
 
 // Include the list of token values lemon generates.
 #include "queryparser_token.h"
@@ -133 +134 @@
     QpQuery(bool m) : q(), match_nothing(m) {}
     Query & get() { return q; }
     const Query & get() const { return q; }
+    Xapian::valueno get_valno() const { return q.internal->get_parameter(); }
 
     /// True iff the query is not empty, and doesn't explicitly match nothing.
     bool can_match() { return (!q.empty() && !match_nothing); }
@@ -160 +162 @@
 #endif
 };
 
+/// A structure identifying a group of filter terms
+struct filter_group_id {
+    /** The prefix of the filter terms.
+     *  This is used for boolean filter terms.
+     */
+    string prefix;
+
+    /** The value number of the filter terms.
+     *  This is used for value range terms.
+     */
+    Xapian::valueno valno;
+
+    /// Make a new filter_group_id for boolean filter terms.
+    explicit filter_group_id(const string & prefix_) : prefix(prefix_), valno(Xapian::BAD_VALUENO) {}
+
+    /// Make a new filter_group_id for value range terms.
+    explicit filter_group_id(Xapian::valueno valno_) : prefix(), valno(valno_) {}
+
+    /// Compare to another filter_group_id.
+    bool operator<(const filter_group_id & other) const {
+        if (prefix != other.prefix) {
+            return prefix < other.prefix;
+        }
+        return valno < other.valno;
+    }
+};
+
 /** Class used to pass information about a token from lexer to parser.
  *
  *  Generally a this class carries term information, but it can be used for the
@@ -177 +206 @@
     bool stem;
     termpos pos;
 
-    std::string make_term() const;
-
   public:
     Term(const string &name_, termpos pos_) : name(name_), stem(false), pos(pos_) { }
     Term(const string &name_) : name(name_), stem(false), pos(0) { }
+    Term(const string &name_, const string &prefix_)
+        : name(name_), prefix(prefix_), stem(false), pos(0) { }
     Term(termpos pos_) : stem(false), pos(pos_) { }
     Term(State * state_, const string &name_, const string &prefix_,
          const string &unstemmed_, bool stem_, termpos pos_)
         : state(state_), name(name_), prefix(prefix_), unstemmed(unstemmed_),
           stem(stem_), pos(pos_) { }
 
+    std::string make_term() const;
+
     void dont_stem() { stem = false; }
 
     termpos get_termpos() const { return pos; }
 
+    filter_group_id get_filter_group_id() const { return filter_group_id(prefix); }
+
     QpQuery * as_query() const { return new QpQuery(make_term(), 1, pos); }
 
     QpQuery * as_wildcarded_query(State * state) const;
@@ -587 +620 @@
                         // until the next space or ')' as part of the boolean
                         // term.
                         it = p;
-                        if (prefix_needs_colon(prefix, *it))
-                            prefix += ':';
-                        string term;
+                        string name;
                         while (it != end && *it > ' ' && *it != ')')
-                            Unicode::append_utf8(term, *it++);
-                        prefix += term;
+                            Unicode::append_utf8(name, *it++);
+                        AutoPtr<Term> term(new Term(name, prefix));
                         field += ':';
-                        field += term;
-                        unstem.insert(make_pair(prefix, field));
-                        Parse(pParser, BOOLEAN_FILTER, new Term(prefix),
-                              &state);
+                        field += name;
+                        unstem.insert(make_pair(term->make_term(), field));
+                        Parse(pParser, BOOLEAN_FILTER, term.release(), &state);
                         continue;
                     }
 
@@ -856 +886 @@
     QpQuery query;
     QpQuery love;
     QpQuery hate;
-    QpQuery filter;
+
+    // filter is a map from prefix to a query for that prefix.  Queries with
+    // the same prefix are combined with OR, and the results of this are
+    // combined with AND to get the full filter.
+    map<filter_group_id, QpQuery> filter;
+
+    QpQuery merge_filters() const {
+        QpQuery q;
+        for (map<filter_group_id, QpQuery>::const_iterator i = filter.begin();
+             i != filter.end(); ++i)
+        {
+            add_to_query(q, Query::OP_AND, i->second.get());
+        }
+        return q;
+    }
 };
 
 class TermList {
@@ -1076 +1120 @@
     // Handle any boolean filters.
     if (!P->filter.empty()) {
         if (E->empty()) {
-            *E = P->filter;
+            *E = P->merge_filters();
             // FIXME and make the query boolean somehow...
         } else {
-            *E = QpQuery(Query::OP_FILTER, *E, P->filter);
+            *E = QpQuery(Query::OP_FILTER, *E, P->merge_filters());
         }
     }
     // FIXME what if E->empty() (all terms are stopwords)?
@@ -1106 +1150 @@
         yy_parse_failed(yypParser);
         return;
     }
+    Xapian::valueno valno = range->get_valno();
     P = new ProbQuery;
-    P->filter = *range;
+    P->filter[filter_group_id(valno)] = *range;
     delete range;
 }
 
@@ -1119 +1164 @@
         yy_parse_failed(yypParser);
         return;
     }
+    Xapian::valueno valno = range->get_valno();
     P = Q;
-    add_to_query(P->filter, Query::OP_AND, *range);
+    add_to_query(P->filter[filter_group_id(valno)], Query::OP_OR, *range);
     delete range;
 }
 
@@ -1190 +1236 @@
 
 prob(P) ::= BOOLEAN_FILTER(T). {
     P = new ProbQuery;
-    P->filter = T->as_query_object();
+    P->filter[T->get_filter_group_id()] = T->as_query_object();
     delete T;
 }
 
 prob(P) ::= stop_prob(Q) BOOLEAN_FILTER(T). {
     P = Q;
-    // FIXME we should OR filters with the same prefix...
-    add_to_query(P->filter, Query::OP_AND, T->as_query_object());
+    // We OR filters with the same prefix...
+    add_to_query(P->filter[T->get_filter_group_id()], Query::OP_OR, T->as_query_object());
     delete T;
 }
 
 prob(P) ::= LOVE BOOLEAN_FILTER(T). {
     // LOVE BOOLEAN_FILTER(T) is just the same as BOOLEAN_FILTER
     P = new ProbQuery;
-    P->filter = T->as_query_object();
+    P->filter[T->get_filter_group_id()] = T->as_query_object();
     delete T;
 }
 
 prob(P) ::= stop_prob(Q) LOVE BOOLEAN_FILTER(T). {
     // LOVE BOOLEAN_FILTER(T) is just the same as BOOLEAN_FILTER
     P = Q;
-    // FIXME we should OR filters with the same prefix...
-    add_to_query(P->filter, Query::OP_AND, T->as_query_object());
+    // We OR filters with the same prefix...
+    add_to_query(P->filter[T->get_filter_group_id()], Query::OP_OR, T->as_query_object());
     delete T;
 }
 

tests/queryparsertest.cc

@@ -532 +532 @@
     { "- NEAR 12V voeding", "(near:(pos=1) OR 12v:(pos=2) OR Zvoed:(pos=3))" },
     { "waarom \"~\" in directorynaam", "(Zwaarom:(pos=1) OR Zin:(pos=2) OR Zdirectorynaam:(pos=3))" },
     { "cd'r NEAR toebehoren", "(cd'r:(pos=1) NEAR 11 toebehoren:(pos=2))" },
+    { "site:1 site:2", "(H1 OR H2)" },
+    { "site:1 site2:2", "(H1 AND J2)" },
+    { "site:1 site:2 site2:2", "((H1 OR H2) AND J2)" },
+    { "site:1 OR site:2", "(H1 OR H2)" },
+    { "site:1 AND site:2", "(H1 AND H2)" },
+#if 0
+    { "A site:1 site:2", "(a FILTER (H1 OR H2))" },
+    { "A (site:1 OR site:2)", "(a FILTER (H1 OR H2))" },
+    { "A (site:1 OR site:2)", "(a FILTER (H1 OR H2))" },
+    { "A site:1 site2:2", "(a FILTER (H1 AND J2))" },
+    { "A site:1 site:2 site2:2", "(a FILTER ((H1 OR H2) AND J2))" },
+    { "A site:1 OR site:2", "(a FILTER (H1 OR H2))" },
+    { "A site:1 AND site:2", "(a FILTER (H1 AND H2))" },
+#endif
+    { "site:xapian.org OR site:www.xapian.org", "(Hxapian.org OR Hwww.xapian.org)" },
+    { "site:xapian.org site:www.xapian.org", "(Hxapian.org OR Hwww.xapian.org)" },
+    { "site:xapian.org AND site:www.xapian.org", "(Hxapian.org AND Hwww.xapian.org)" },
+    { "Xapian site:xapian.org site:www.xapian.org", "(xapian:(pos=1) FILTER (Hxapian.org OR Hwww.xapian.org))" },
+    { "author:richard author:olly writer:charlie", "(ZArichard:(pos=1) OR ZAolli:(pos=2) OR ZAcharli:(pos=3))"},
     { NULL, NULL }
 };
 
@@ -573 +592 @@
     queryparser.set_stemmer(Xapian::Stem("english"));
     queryparser.set_stemming_strategy(Xapian::QueryParser::STEM_SOME);
     queryparser.add_prefix("author", "A");
+    queryparser.add_prefix("writer", "A");
     queryparser.add_prefix("title", "XT");
     queryparser.add_prefix("subject", "XT");
     queryparser.add_boolean_prefix("site", "H");
+    queryparser.add_boolean_prefix("site2", "J");
     for (test *p = test_or_queries; p->query; ++p) {
         string expect, parsed;
         if (p->expect)
@@ -977 +998 @@
     { "hello a..b", "(hello:(pos=1) FILTER VALUE_RANGE 1 a b)" },
     { "hello a..b world", "((hello:(pos=1) OR world:(pos=2)) FILTER VALUE_RANGE 1 a b)" },
     { "hello a..b test:foo", "(hello:(pos=1) FILTER (VALUE_RANGE 1 a b AND XTESTfoo))" },
+    { "hello a..b test:foo test:bar", "(hello:(pos=1) FILTER (VALUE_RANGE 1 a b AND (XTESTfoo OR XTESTbar)))" },
+    { "hello a..b c..d test:foo", "(hello:(pos=1) FILTER ((VALUE_RANGE 1 a b OR VALUE_RANGE 1 c d) AND XTESTfoo))" },
+    { "hello a..b c..d test:foo test:bar", "(hello:(pos=1) FILTER ((VALUE_RANGE 1 a b OR VALUE_RANGE 1 c d) AND (XTESTfoo OR XTESTbar)))" },
     { "-5..7", "VALUE_RANGE 1 -5 7" },
     { "hello -5..7", "(hello:(pos=1) FILTER VALUE_RANGE 1 -5 7)" },
     { "-5..7 hello", "(hello:(pos=1) FILTER VALUE_RANGE 1 -5 7)" },
@@ -1030 +1054 @@
     { "12/03/99..12/04/01", "VALUE_RANGE 1 19990312 20010412" },
     { "03-12-99..04-14-01", "VALUE_RANGE 1 19990312 20010414" },
     { "(test:a..test:b hello)", "(hello:(pos=1) FILTER VALUE_RANGE 3 test:a test:b)" },
+    { "12..42kg 5..6kg 1..12", "(VALUE_RANGE 2 1 12 AND (VALUE_RANGE 5 12 42 OR VALUE_RANGE 5 5 6))" },
     { NULL, NULL }
 };
 

include/xapian/query.h

@@ -377 +377 @@
          */
         std::string get_description() const;
 
+        /** Get the numeric parameter used in this query.
+         *
+         *  This is used by the queryparser to get the value number for
+         *  VALUE_RANGE queries.  It should be replaced by a public method on
+         *  the Query class at some point, but the API which should be used for
+         *  that is unclear, so this is a temporary workaround.
+         */
+        Xapian::termcount get_parameter() const {  return parameter; }
+
         /** Get the length of the query, used by some ranking formulae.
          *  This value is calculated automatically - if you want to override
          *  it you can pass a different value to Enquire::set_query().

include/xapian/queryparser.h

@@ -238 +238 @@
      *  will be converted to Hxapian.org combined with any probabilistic
      *  query with OP_FILTER.
      *
-     *  Multiple fields can be mapped to the same prefix (so you can
-     *  e.g. make site: and domain: aliases for each other).
+     *  If multiple boolean filters are specified in a query for the same
+     *  prefix, they will be combined with the OR operator.  Then, if there are
+     *  boolean filters for different prefixes, they will be combined with the
+     *  AND operator.
      *
+     *  Multiple fields can be mapped to the same prefix (so you can e.g. make
+     *  site: and domain: aliases for each other).  Instances of fields with
+     *  different aliases but the same prefix will still be combined with the
+     *  OR operator.
+     *
+     *  For example, if "site" and "domain" map to "H", but author maps to "A",
+     *  a search for "site:Foo domain:Bar author:Fred" will map to
+     *  "(Hfoo OR Hbar) AND Afred".
+     *
      *  @param field   The user visible field name
      *  @param prefix  The term prefix to map this to
      */