Ticket #157: boolgroups.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"
@@ -100 +101 @@
     /// Flag, true iff this represents a "MatchNothing" query.
     bool match_nothing;
 
+    /// The value number that this represents, if it's a range query.
+    /// (Needed, since we can't get the value number back out of the "q" member.)
+    Xapian::valueno valno;
+
   public:
     QpQuery(const QpQuery & tocopy)
-        : q(tocopy.q), match_nothing(tocopy.match_nothing)
+        : q(tocopy.q), match_nothing(tocopy.match_nothing),
+          valno(Xapian::BAD_VALUENO)
     {}
 
     QpQuery & operator=(const QpQuery & tocopy)
@@ -115 +121 @@
     /** A query consisting of a single term. */
     QpQuery(const std::string & tname, Xapian::termcount wqf,
             Xapian::termpos pos)
-        : q(tname, wqf, pos), match_nothing(false)
+        : q(tname, wqf, pos), match_nothing(false),
+          valno(Xapian::BAD_VALUENO)
     {}
 
     /** A query consisting of two subqueries, combined with operator op. */
     QpQuery(Query::op op, const QpQuery & left, const QpQuery & right)
-        : q(op, left.get(), right.get()), match_nothing(false)
+        : q(op, left.get(), right.get()), match_nothing(false),
+          valno(Xapian::BAD_VALUENO)
     {}
 
-    QpQuery(Query::op op_, Xapian::valueno valno,
+    QpQuery(Query::op op_, Xapian::valueno valno_,
           const std::string &begin, const std::string &end)
-        : q(op_, valno, begin, end), match_nothing(false)
+        : q(op_, valno_, begin, end), match_nothing(false),
+          valno(valno_)
     {}
 
-    QpQuery(const Query & q_) : q(q_), match_nothing(false) {}
-    QpQuery() : q(), match_nothing(false) {}
-    QpQuery(bool m) : q(), match_nothing(m) {}
+    QpQuery(const Query & q_) : q(q_), match_nothing(false), valno(Xapian::BAD_VALUENO) {}
+    QpQuery() : q(), match_nothing(false), valno(Xapian::BAD_VALUENO) {}
+    QpQuery(bool m) : q(), match_nothing(m), valno(Xapian::BAD_VALUENO) {}
     Query & get() { return q; }
     const Query & get() const { return q; }
+    Xapian::valueno get_valno() const { return valno; }
 
     /// True iff the query is not empty, and doesn't explicitly match nothing.
     bool can_match() { return (!q.empty() && !match_nothing); }
@@ -160 +170 @@
 #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 +214 @@
     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;
@@ -589 +630 @@
                         // 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;
                     }
 
@@ -858 +896 @@
     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 {
@@ -1078 +1130 @@
     // 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)?
@@ -1108 +1160 @@
         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;
 }
 
@@ -1121 +1174 @@
         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;
 }
 
@@ -1192 +1246 @@
 
 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/harness/testsuite.h

@@ -225 +225 @@
 
 /// Test for equality of two things.
 #define TEST_EQUAL(a, b) TEST_AND_EXPLAIN(((a) == (b)), \
-        "Expected `"STRINGIZE(a)"' and `"STRINGIZE(b)"' to be equal:" \
-        " were " << (a) << " and " << (b))
+        "Expected `"STRINGIZE(a)"' and `"STRINGIZE(b)"' to be equal:\n" \
+        " were " << (a) << "\n  and " << (b))
 
 /** Test for equality of two strings.
  *

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))" },
     { NULL, NULL }
 };
 
@@ -576 +594 @@
     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)
@@ -969 +988 @@
     { "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)" },
@@ -1020 +1042 @@
     { "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 }
 };