Ticket #203: patch

tests/api_nodb.cc

@@ -416 +416 @@
     return true;
 }
 
+// Test various scaleweight queries to check that they collapse correctly.
+static bool test_scaleweight5()
+{
+    Xapian::Query queries1[3] = {
+        Xapian::Query("wibble"),
+        Xapian::Query("wobble"),
+        Xapian::Query(Xapian::Query::OP_OR, string("jelly"), string("belly"))
+    };
 
+    double multipliers[3] = {
+        1.5, 1, 0
+    };
+    for (int i = 0; i < 3; ++i) {
+        double multiplier = multipliers[i];
+        vector<Xapian::Query> vec1;
+        for (int j = 0; j < 3; ++j) {
+            vec1.push_back(Xapian::Query(Xapian::Query::OP_SCALE_WEIGHT,
+                                         queries1[j], multiplier));
+        }
+
+        Xapian::Query myquery1(Xapian::Query::OP_OR, vec1.begin(), vec1.end());
+
+        if (multiplier == 1) {
+            TEST_EQUAL(myquery1.get_description(),
+                       "Xapian::Query((wibble OR wobble OR jelly OR belly))");
+        } else {
+            TEST_EQUAL(myquery1.get_description(),
+                       "Xapian::Query(((wibble OR wobble OR jelly OR belly) * "
+                       + om_tostring(multiplier) + "))");
+        }
+    }
+    return true;
+}
+
+// Test that scaleweight queries combine and simplify correctly.
+static bool test_scaleweight6()
+{
+    Xapian::Query foo("foo");
+    Xapian::Query bar("bar");
+    Xapian::Query foo_7(Xapian::Query::OP_SCALE_WEIGHT, foo, 7);
+    Xapian::Query bar_7(Xapian::Query::OP_SCALE_WEIGHT, bar, 7);
+
+    // Foo * 7 * 1/7 == Foo
+    Xapian::Query foo_7_div_7(Xapian::Query::OP_SCALE_WEIGHT, foo_7, 1.0/7);
+    TEST_EQUAL(foo_7_div_7.get_description(), "Xapian::Query(foo)");
+
+    // Foo * 7 * 10 == Foo * 70
+    Xapian::Query foo_7_10(Xapian::Query::OP_SCALE_WEIGHT, foo_7, 10);
+    TEST_EQUAL(foo_7_10.get_description(),
+               "Xapian::Query((foo * 70))");
+
+    // (Foo * 7 OR Bar * 7) == (Foo OR Bar) * 7
+    Xapian::Query foo_7_bar_7(Xapian::Query::OP_OR, foo_7, bar_7);
+    TEST_EQUAL(foo_7_bar_7.get_description(), "Xapian::Query(((foo OR bar) * 7))");
+
+    // (Foo * 7 OR Foo * 7) == (Foo OR Foo) * 7
+    Xapian::Query foo_7_foo_7(Xapian::Query::OP_OR, foo_7, foo_7);
+    TEST_EQUAL(foo_7_foo_7.get_description(), "Xapian::Query((foo:(wqf=2) * 7))");
+
+    // (Foo * 7 OR Bar * 7) * 10 == (Foo OR Bar) * 70
+    Xapian::Query foo_7_bar_7_10(Xapian::Query::OP_SCALE_WEIGHT,
+                                 Xapian::Query(Xapian::Query::OP_OR,
+                                               foo_7, bar_7),
+                                 10);
+    TEST_EQUAL(foo_7_bar_7_10.get_description(),
+               "Xapian::Query(((foo OR bar) * 70))");
+
+    // (Foo * 7 OR Foo * 7) * 10 == (Foo, wqf=2) * 70
+    Xapian::Query foo_7_foo_7_10(Xapian::Query::OP_SCALE_WEIGHT,
+                                 Xapian::Query(Xapian::Query::OP_OR,
+                                               foo_7, foo_7),
+                                 10);
+    TEST_EQUAL(foo_7_foo_7_10.get_description(),
+               "Xapian::Query((foo:(wqf=2) * 70))");
+
+
+    // (Foo * 7 OR Bar) * 10 == (Foo * 7 OR Bar) * 10
+    Xapian::Query foo_7_bar_10(Xapian::Query::OP_SCALE_WEIGHT,
+                               Xapian::Query(Xapian::Query::OP_OR,
+                                             foo_7, bar),
+                               10);
+    TEST_EQUAL(foo_7_bar_10.get_description(),
+               "Xapian::Query((((foo * 7) OR bar) * 10))");
+
+    // (Foo * 7) * 0 == Foo * 0
+    Xapian::Query foo_7_0(Xapian::Query::OP_SCALE_WEIGHT, foo_7, 0);
+    TEST_EQUAL(foo_7_0.get_description(),
+               "Xapian::Query((foo * 0))");
+
+    // Factor is a double which, when multiplied by its reciprocal, doesn't
+    // give exactly 1.0
+    double factor = 179.76931348623157e306;
+
+    Xapian::Query foo_factor(Xapian::Query::OP_SCALE_WEIGHT, foo, factor);
+    Xapian::Query foo_factor_inv(Xapian::Query::OP_SCALE_WEIGHT, foo_factor, 1.0 / factor);
+    TEST_EQUAL(foo_factor_inv.get_description(), "Xapian::Query(foo)");
+
+    return true;
+}
+
+
 // #######################################################################
 // # End of test cases: now we list the tests to run.
 
@@ -441 +541 @@
     TESTCASE(stringlistserialise1),
     TESTCASE(scaleweight3),
     TESTCASE(scaleweight4),
+    TESTCASE(scaleweight5),
+    TESTCASE(scaleweight6),
     END_OF_TESTCASES
 };

include/xapian/query.h

@@ -327 +327 @@
          */
         bool simplify_matchnothing();
 
+        /** Pull up any common scale weight nodes.
+         */
+        void pull_up_scale_weight();
+
         /** Get a string describing the given query type.
          */
         static std::string get_op_name(Xapian::Query::Internal::op_t op);

api/omqueryinternal.cc

@@ -705 +705 @@
     return false;
 }
 
+void
+Xapian::Query::Internal::pull_up_scale_weight()
+{
+    Assert(subqs.size() >= 1);
+
+    // If our operator is not OP_SCALE_WEIGHT, there's no point in "pulling up"
+    // a single OP_SCALE_WEIGHT subquery.
+    if (subqs.size() == 1 && op != OP_SCALE_WEIGHT)
+        return;
+
+    // Check if all subqueries are OP_SCALE_WEIGHT
+    // FIXME - in future, we might want to pull up weights if a majority of the
+    // subqueries are OP_SCALE_WEIGHT with the same multiplier, but for now we
+    // take the simpler approach of only doing it if they all are.
+    subquery_list::iterator sq;
+    for (sq = subqs.begin(); sq != subqs.end(); ++sq) {
+        if ((*sq)->op != OP_SCALE_WEIGHT)
+            return;
+    }
+
+    // All subqueries are OP_SCALE_WEIGHT - check if they have exactly the same
+    // weight.
+    sq = subqs.begin();
+    double common_subweight = (*sq)->dbl_parameter;
+    ++sq;
+    for (; sq != subqs.end(); ++sq) {
+        if ((*sq)->dbl_parameter != common_subweight)
+            return;
+    }
+
+    // Remove the OP_SCALE_WEIGHT nodes from the subqueries.
+    // Do this by building a new query, so that any subqueries get distributed
+    // (since this is done inside add_subquery()).
+    Xapian::Query::Internal newq(*this);
+    for (sq = newq.subqs.begin(); sq != newq.subqs.end(); sq++) {
+        delete *sq;
+    }
+    newq.subqs.clear();
+    for (sq = subqs.begin(); sq != subqs.end(); ++sq) {
+        Assert((*sq)->op == OP_SCALE_WEIGHT);
+        Assert((*sq)->subqs.size() == 1);
+        newq.add_subquery((*sq)->subqs.front());
+    }
+    Xapian::Query::Internal * newqptr = newq.end_construction();
+    Assert(newqptr);
+    this->swap(*newqptr);
+
+    if (op == OP_SCALE_WEIGHT) {
+        // Combine the common weight with our weight.
+        dbl_parameter *= common_subweight;
+    } else {
+        // Make a new OP_SCALE_WEIGHT node with the common weight, and put it
+        // above ourself.
+        Xapian::Query::Internal newq2(OP_SCALE_WEIGHT, 0);
+        newq2.dbl_parameter = common_subweight;
+        newq2.add_subquery(this);
+        newqptr = newq2.end_construction();
+        Assert(newqptr);
+        this->swap(*newqptr);
+    }
+}
+
 Xapian::Query::Internal *
 Xapian::Query::Internal::simplify_query()
 {
@@ -715 +777 @@
         return 0;
     }
 
+    if (op != OP_LEAF && op != OP_VALUE_RANGE) {
+        // Pull any scale weight operators with the same weight.
+        pull_up_scale_weight();
+    }
+
     // General simplifications, dependent on operator.
     switch (op) {
         case OP_LEAF: