From 1d733f2830dad39bab7368bf5e8000fd6fb29c5d Mon Sep 17 00:00:00 2001 From: Olly Betts Date: Thu, 21 Sep 2017 09:44:12 +1200 Subject: [PATCH] replace_document: Only force load values for same docid There's no need to force load unmodified values in other cases, and it should be a bit more efficient not to. --- xapian-core/backends/glass/glass_values.cc | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/xapian-core/backends/glass/glass_values.cc b/xapian-core/backends/glass/glass_values.cc index 9f509a1ba..e235ca859 100644 --- a/xapian-core/backends/glass/glass_values.cc +++ b/xapian-core/backends/glass/glass_values.cc @@ -454,10 +454,22 @@ GlassValueManager::replace_document(Xapian::docid did, const Xapian::Document &doc, map & value_stats) { - // Load the values into the document from the database, if they haven't - // been already. (If we don't do this before deleting the old values, - // replacing a document with itself will lose the values.) - doc.internal->need_values(); + if (doc.get_docid() == did) { + // If we're replacing a document with itself, but the optimisation for + // this higher up hasn't kicked in (e.g. because we've added/replaced + // a document since this one was read) and the values haven't changed, + // then the call to delete_document() below will remove the values + // before the subsequent add_document() can read them. + // + // The simplest way to handle this is to force the document to read its + // values, which we only need to do this is the docid matches. Note that + // this check can give false positives as we don't also check the + // database, so for example replacing document 4 in one database with + // document 4 from another will unnecessarily trigger this, but forcing + // the values to be read is fairly harmless, and this is unlikely to be + // a common case. + doc.internal->need_values(); + } delete_document(did, value_stats); add_document(did, doc, value_stats); } -- 2.11.4.GIT