Ticket #3: db-error.diff

xapian-core/api/editdistance.h

@@ +1 @@
+
 /** @file editdistance.h
  * @brief Edit distance calculation algorithm.
  */

xapian-core/api/omdatabase.cc

@@ -740 +740 @@
     RETURN(uuid);
 }
 
+void
+Database::set_error_handler(ErrorHandler* error_handler_)
+{
+    LOGCALL(API, std::string, "Database::set_error_handler", error_handler_);
+    vector<intrusive_ptr<Database::Internal> >::const_iterator i;
+    for (i = internal.begin(); i != internal.end(); ++i) {
+        (*i)->error_handler = error_handler_;
+    }
+}
+
+
 ///////////////////////////////////////////////////////////////////////////
 
 WritableDatabase::WritableDatabase() : Database()

xapian-core/api/omenquire.cc

@@ -680 +680 @@
                        collapse_max, collapse_key,
                        percent_cutoff, weight_cutoff,
                        order, sort_key, sort_by, sort_value_forward,
-                       errorhandler, stats, weight, spies,
+                       stats, weight, spies,
                        (sorter != NULL),
                        (mdecider != NULL));
     // Run query and put results into supplied Xapian::MSet object.
@@ -829 +829 @@
 void
 Enquire::Internal::request_doc(const Xapian::Internal::MSetItem &item) const
 {
-    try {
-        unsigned int multiplier = db.internal.size();
+    unsigned int multiplier = db.internal.size();
 
-        Xapian::docid realdid = (item.did - 1) / multiplier + 1;
-        Xapian::doccount dbnumber = (item.did - 1) % multiplier;
+    Xapian::docid realdid = (item.did - 1) / multiplier + 1;
+    Xapian::doccount dbnumber = (item.did - 1) % multiplier;
 
+    try {
         db.internal[dbnumber]->request_document(realdid);
     } catch (Error & e) {
-        if (errorhandler) (*errorhandler)(e);
+        ErrorHandler* error_handler = db.internal[dbnumber]->error_handler;
+        if (error_handler) (*error_handler)(e);
         throw;
     }
 }
@@ -845 +846 @@
 Document
 Enquire::Internal::read_doc(const Xapian::Internal::MSetItem &item) const
 {
-    try {
-        unsigned int multiplier = db.internal.size();
+    unsigned int multiplier = db.internal.size();
 
-        Xapian::docid realdid = (item.did - 1) / multiplier + 1;
-        Xapian::doccount dbnumber = (item.did - 1) % multiplier;
+    Xapian::docid realdid = (item.did - 1) / multiplier + 1;
+    Xapian::doccount dbnumber = (item.did - 1) % multiplier;
 
+    try {
         Xapian::Document::Internal *doc;
         doc = db.internal[dbnumber]->collect_document(realdid);
         return Document(doc);
     } catch (Error & e) {
-        if (errorhandler) (*errorhandler)(e);
+        ErrorHandler* error_handler = db.internal[dbnumber]->error_handler;
+        if (error_handler) (*error_handler)(e);
         throw;
     }
 }
@@ -902 +904 @@
 Enquire::get_query() const
 {
     LOGCALL(API, const Xapian::Query &, "Xapian::Enquire::get_query", NO_ARGS);
-    try {
-        RETURN(internal->get_query());
-    } catch (Error & e) {
-        if (internal->errorhandler) (*internal->errorhandler)(e);
-        throw;
-    }
+    RETURN(internal->get_query());
 }
 
 void

xapian-core/backends/database.cc

@@ -49 +49 @@
     // For the normal case of local databases, nothing needs to be done.
 }
 
-
 Xapian::doccount
 Database::Internal::get_value_freq(Xapian::valueno) const
 {

xapian-core/backends/database.h

@@ -33 +33 @@
 #include <xapian/types.h>
 #include <xapian/database.h>
 #include <xapian/document.h>
+#include <xapian/errorhandler.h>
 #include <xapian/positioniterator.h>
 #include <xapian/termiterator.h>
 #include <xapian/valueiterator.h>
@@ -75 +76 @@
         bool transaction_active() const { return int(transaction_state) > 0; }
 
         /** Create a database - called only by derived classes. */
-        Internal() : transaction_state(TRANSACTION_NONE) { }
+        Internal() : transaction_state(TRANSACTION_NONE), error_handler(NULL) { }
 
         /** Internal method to perform cleanup when a writable database is
          *  destroyed with uncommitted changes.
@@ -108 +109 @@
          */
         virtual void keep_alive();
 
+        /** Internal pointer to the database's ErrorHandler
+         */
+        ErrorHandler* error_handler;
+
         //////////////////////////////////////////////////////////////////
         // Database statistics:
         // ====================

xapian-core/include/xapian/database.h

@@ -30 +30 @@
 #include <vector>
 
 #include <xapian/attributes.h>
+#include <xapian/errorhandler.h>
 #include <xapian/intrusive_ptr.h>
 #include <xapian/types.h>
 #include <xapian/positioniterator.h>
@@ -508 +509 @@
          *  @param opts Options to use for check
          */
         static size_t check(const std::string & path, int opts);
+
+        /** Set an ErrorHandler for database. This will be applied to all
+         *  databases in internal.
+         *
+         *  @param error_handler_ ErrorHandler* to use
+         */
+        void set_error_handler(ErrorHandler* error_handler_);
+
 };
 
 /** This class provides read/write access to a database.

xapian-core/include/xapian/enquire.h

@@ -711 +711 @@
 
         /** Get the query which has been set.
          *  This is only valid after set_query() has been called.
-         *
-         *  @exception Xapian::InvalidArgumentError will be thrown if query has
-         *             not yet been set.
          */
         const Xapian::Query & get_query() const;
 

xapian-core/matcher/mergepostlist.cc

@@ -30 +30 @@
 #include "debuglog.h"
 #include "omassert.h"
 #include "valuestreamdocument.h"
-#include "xapian/errorhandler.h"
 
 // NB don't prune - even with one sublist we still translate docids...
 
@@ -59 +58 @@
             if (unsigned(current) >= plists.size()) break;
             vsdoc.new_subdb(current);
         } catch (Xapian::Error & e) {
+            //TODO: add test coverage
+            Xapian::ErrorHandler* errorhandler = handlers[current];
             if (errorhandler) {
                 LOGLINE(EXCEPTION, "Calling error handler in MergePostList::next().");
                 (*errorhandler)(e);
@@ -176 +177 @@
             double w = (*i)->recalc_maxweight();
             if (w > w_max) w_max = w;
         } catch (Xapian::Error & e) {
-            if (errorhandler) {
+            //TODO: add test coverage
+            Xapian::ErrorHandler* errorhandler_ = handlers[current];
+            if (errorhandler_) {
                 LOGLINE(EXCEPTION, "Calling error handler in MergePostList::recalc_maxweight().");
-                (*errorhandler)(e);
+                (*errorhandler_)(e);
 
                 if (current == i - plists.begin()) {
                     // Fatal error

xapian-core/matcher/mergepostlist.h

@@ -26 +26 @@
 #define OM_HGUARD_MERGEPOSTLIST_H
 
 #include "api/postlist.h"
+#include "xapian/errorhandler.h"
 
 class MultiMatch;
 class ValueStreamDocument;
@@ -59 +60 @@
          */
         ValueStreamDocument & vsdoc;
 
-        Xapian::ErrorHandler * errorhandler;
+        /**
+         *
+         */
+        std::vector<Xapian::ErrorHandler *> handlers;
+
     public:
         Xapian::termcount get_wdf() const;
         Xapian::doccount get_termfreq_max() const;
@@ -90 +95 @@
         MergePostList(const std::vector<PostList *> & plists_,
                       MultiMatch *matcher_,
                       ValueStreamDocument & vsdoc_,
-                      Xapian::ErrorHandler * errorhandler_)
+                      std::vector<Xapian::ErrorHandler *> & handlers_)
             : plists(plists_), current(-1), matcher(matcher_), vsdoc(vsdoc_),
-              errorhandler(errorhandler_) { }
+            handlers(handlers_)
+            { }
 
         ~MergePostList();
 };

xapian-core/matcher/multimatch.cc

@@ -136 +136 @@
  */
 static void
 prepare_sub_matches(vector<intrusive_ptr<SubMatch> > & leaves,
-                    Xapian::ErrorHandler * errorhandler,
-                    Xapian::Weight::Internal & stats)
+                    Xapian::Weight::Internal & stats,
+                    const Xapian::Database & db)
 {
-    LOGCALL_STATIC_VOID(MATCH, "prepare_sub_matches", leaves | errorhandler | stats);
+    LOGCALL_STATIC_VOID(MATCH, "prepare_sub_matches", leaves | stats);
     // We use a vector<bool> to track which SubMatches we're already prepared.
     vector<bool> prepared;
     prepared.resize(leaves.size(), false);
@@ -155 +155 @@
                     --unprepared;
                 }
             } catch (Xapian::Error & e) {
-                if (!errorhandler) throw;
+                //TODO:add test coverage
+                Xapian::ErrorHandler* errorhandler_ = db.internal[leaf]->error_handler;
+                if (!errorhandler_) throw;
 
                 LOGLINE(EXCEPTION, "Calling error handler for prepare_match() on a SubMatch.");
-                (*errorhandler)(e);
+                (*errorhandler_)(e);
                 // Continue match without this sub-match.
                 leaves[leaf] = NULL;
                 prepared[leaf] = true;
@@ -211 +213 @@
                        Xapian::valueno sort_key_,
                        Xapian::Enquire::Internal::sort_setting sort_by_,
                        bool sort_value_forward_,
-                       Xapian::ErrorHandler * errorhandler_,
                        Xapian::Weight::Internal & stats,
                        const Xapian::Weight * weight_,
                        const vector<Xapian::MatchSpy *> & matchspies_,
@@ -222 +223 @@
           order(order_),
           sort_key(sort_key_), sort_by(sort_by_),
           sort_value_forward(sort_value_forward_),
-          errorhandler(errorhandler_), weight(weight_),
+          weight(weight_),
           is_remote(db.internal.size()),
           matchspies(matchspies_)
 {
-    LOGCALL_CTOR(MATCH, "MultiMatch", db_ | query_ | qlen | omrset | collapse_max_ | collapse_key_ | percent_cutoff_ | weight_cutoff_ | int(order_) | sort_key_ | int(sort_by_) | sort_value_forward_ | errorhandler_ | stats | weight_ | matchspies_ | have_sorter | have_mdecider);
+    LOGCALL_CTOR(MATCH, "MultiMatch", db_ | query_ | qlen | omrset | collapse_max_ | collapse_key_ | percent_cutoff_ | weight_cutoff_ | int(order_) | sort_key_ | int(sort_by_) | sort_value_forward_ | stats | weight_ | matchspies_ | have_sorter | have_mdecider);
 
     if (query.empty()) return;
 
@@ -267 +268 @@
             smatch = new LocalSubMatch(subdb, query, qlen, subrsets[i], weight);
 #endif /* XAPIAN_HAS_REMOTE_BACKEND */
         } catch (Xapian::Error & e) {
-            if (!errorhandler) throw;
+            Xapian::ErrorHandler* errorhandler_ = subdb->error_handler;
+            if (!errorhandler_) throw;
             LOGLINE(EXCEPTION, "Calling error handler for creation of a SubMatch from a database and query.");
-            (*errorhandler)(e);
+            (*errorhandler_)(e);
             // Continue match without this sub-postlist.
             smatch = NULL;
         }
@@ -277 +279 @@
     }
 
     stats.mark_wanted_terms(query);
-    prepare_sub_matches(leaves, errorhandler, stats);
+    prepare_sub_matches(leaves, stats, db);
     stats.set_bounds_from_db(db);
 }
 
@@ -333 +335 @@
     // Start matchers.
     {
         vector<intrusive_ptr<SubMatch> >::iterator leaf;
-        for (leaf = leaves.begin(); leaf != leaves.end(); ++leaf) {
-            if (!(*leaf).get()) continue;
+        for (size_t i = 0; i < leaves.size(); ++i) {
+            if (!(leaves[i]).get()) continue;
             try {
-                (*leaf)->start_match(0, first + maxitems,
+                (leaves[i])->start_match(0, first + maxitems,
                                      first + check_at_least, stats);
             } catch (Xapian::Error & e) {
-                if (!errorhandler) throw;
+                //TODO: add test coverage
+                Xapian::ErrorHandler* errorhandler_ = db.internal[i]->error_handler;
+                if (!errorhandler_) throw;
                 LOGLINE(EXCEPTION, "Calling error handler for "
                                    "start_match() on a SubMatch.");
-                (*errorhandler)(e);
+                (*errorhandler_)(e);
                 // Continue match without this sub-match.
-                *leaf = NULL;
+                leaves[i] = NULL;
             }
         }
     }
@@ -364 +368 @@
     for (size_t i = 0; i != leaves.size(); ++i) {
         PostList *pl;
         try {
+            if (!leaves[i].get()) {
+                pl = new EmptyPostList;
+                postlists.push_back(pl);
+                continue;
+            }
             pl = leaves[i]->get_postlist_and_term_info(this,
                                                        termfreqandwts_ptr,
                                                        &total_subqs);
@@ -380 +389 @@
                 }
             }
         } catch (Xapian::Error & e) {
-            if (!errorhandler) throw;
+            //TODO: add test coverage
+            Xapian::ErrorHandler* errorhandler_ = db.internal[i]->error_handler;
+            if (!errorhandler_) throw;
             LOGLINE(EXCEPTION, "Calling error handler for "
                                "get_term_info() on a SubMatch.");
-            (*errorhandler)(e);
+            (*errorhandler_)(e);
             // FIXME: check if *ALL* the remote servers have failed!
             // Continue match without this sub-match.
             leaves[i] = NULL;
@@ -396 +407 @@
     ValueStreamDocument vsdoc(db);
     ++vsdoc._refs;
     Xapian::Document doc(&vsdoc);
+    vector<Xapian::ErrorHandler*>handlers;
 
+    Xapian::doccount number_of_subdbs = db.internal.size();
+    for (size_t i = 0; i != number_of_subdbs; ++i) {
+        Xapian::Database::Internal *subdb = db.internal[i].get();
+        Assert(subdb);
+        handlers.push_back(subdb->error_handler);
+    }
     // Get a single combined postlist
     AutoPtr<PostList> pl;
     if (postlists.size() == 1) {
         pl.reset(postlists.front());
     } else {
-        pl.reset(new MergePostList(postlists, this, vsdoc, errorhandler));
+        pl.reset(new MergePostList(postlists, this, vsdoc, handlers));
     }
 
     LOGLINE(MATCH, "pl = (" << pl->get_description() << ")");

xapian-core/matcher/multimatch.h

@@ -57 +57 @@
 
         bool sort_value_forward;
 
-        /// ErrorHandler
-        Xapian::ErrorHandler * errorhandler;
-
         /// Weighting scheme
         const Xapian::Weight * weight;
 
@@ -93 +90 @@
          *  @param query     The query
          *  @param qlen      The query length
          *  @param omrset    The relevance set (or NULL for no RSet)
-         *  @param errorhandler Errorhandler object
          *  @param stats     The stats object to add our stats to.
          *  @param wtscheme  Weighting scheme
          *  @param matchspies_ Any the MatchSpy objects in use.
@@ -112 +108 @@
                    Xapian::valueno sort_key_,
                    Xapian::Enquire::Internal::sort_setting sort_by_,
                    bool sort_value_forward_,
-                   Xapian::ErrorHandler * errorhandler,
                    Xapian::Weight::Internal & stats,
                    const Xapian::Weight *wtscheme,
                    const vector<Xapian::MatchSpy *> & matchspies_,

xapian-core/net/remoteserver.cc

@@ -459 +459 @@
     Xapian::Weight::Internal local_stats;
     MultiMatch match(*db, query, qlen, &rset, collapse_max, collapse_key,
                      percent_cutoff, weight_cutoff, order,
-                     sort_key, sort_by, sort_value_forward, NULL,
+                     sort_key, sort_by, sort_value_forward,
                      local_stats, wt.get(), matchspies.spies, false, false);
 
     send_message(REPLY_STATS, serialise_stats(local_stats));

xapian-core/tests/api_db.cc

@@ -226 +226 @@
     return true;
 }
 
-#if 0 // the "force error" mechanism is no longer in place...
+
 class MyErrorHandler : public Xapian::ErrorHandler {
     public:
         int count;
@@ -241 +241 @@
         MyErrorHandler() : count (0) {}
 };
 
+DEFINE_TESTCASE(errorhandlercalled, remote && !multi) {
+    MyErrorHandler myhandler;
+    BackendManagerLocal local_manager;
+    Xapian::Database dbs;
+
+    dbs.add_database(get_remote_database("apitest_simpledata", 5000));
+    dbs.add_database(local_manager.get_database("apitest_simpledata"));
+
+    dbs.set_error_handler(&myhandler);
+    Xapian::Enquire enquire(dbs);
+
+    enquire.set_query(Xapian::Query("word"));
+    Xapian::MultiValueKeyMaker sorter;
+
+    enquire.set_sort_by_key_then_relevance(&sorter, true);
+    Xapian::MSet mset = enquire.get_mset(0, 25);
+    TEST_EQUAL(myhandler.count, 1);
+    TEST_EQUAL(mset.size(), 2);
+    return true;
+}
+
+#if 0 // the "force error" mechanism is no longer in place...
 // tests error handler in multimatch().
 DEFINE_TESTCASE(multierrhandler1, backend) {
     MyErrorHandler myhandler;
 
     Xapian::Database mydb2(get_database("apitest_simpledata"));
     Xapian::Database mydb3(get_database("apitest_simpledata2"));
+
     int errcount = 1;
     for (int testcount = 0; testcount < 14; testcount ++) {
         tout << "testcount=" << testcount << "\n";
@@ -332 +355 @@
                 break;
         }
         tout << "db=" << dbs << "\n";
-        Xapian::Enquire enquire(dbs, &myhandler);
+        dbs.set_error_handler(myhandler);
+        Xapian::Enquire enquire(dbs);
 
         // make a query
         Xapian::Query myquery = query(Xapian::Query::OP_OR, "inmemory", "word");