From 22b84f7181a37c8f71882a65a52d0ac4dec9dcc6 Mon Sep 17 00:00:00 2001
From: Olly Betts <olly@survex.com>
Date: Wed, 31 Oct 2018 04:58:51 +1300
Subject: [PATCH] Fix bug in split position handling

When we look to see in the new position is present before the split
we were only checking if the std::lower_bound() (binary chop) ended
up on a valid position, but we also need to check if that position
is the one we're trying to add.

This was leading to positions getting lost in this situation.

(cherry picked from commit edc381036725373818fa04d137a69f5941a95150)
---
 xapian-core/api/omdocument.cc    |  2 +-
 xapian-core/tests/api_backend.cc | 42 ++++++++++++++++++++++------------------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/xapian-core/api/omdocument.cc b/xapian-core/api/omdocument.cc
index d0ee634d2..70a217358 100644
--- a/xapian-core/api/omdocument.cc
+++ b/xapian-core/api/omdocument.cc
@@ -275,7 +275,7 @@ OmDocumentTerm::add_position(Xapian::termcount wdf_inc, Xapian::termpos tpos)
 	    auto i = lower_bound(positions.cbegin(),
 				 positions.cbegin() + split,
 				 tpos);
-	    if (i != positions.cbegin() + split)
+	    if (i != positions.cbegin() + split && *i == tpos)
 		return false;
 	}
 	positions.push_back(tpos);
diff --git a/xapian-core/tests/api_backend.cc b/xapian-core/tests/api_backend.cc
index a8253c567..b9ee4b614 100644
--- a/xapian-core/tests/api_backend.cc
+++ b/xapian-core/tests/api_backend.cc
@@ -1689,27 +1689,31 @@ DEFINE_TESTCASE(nodocs1, transactions && !remote) {
     return true;
 }
 
-/** Regression test for bug fixed in 1.4.8.
- *
- *  Chert and glass each has inline functions make_slot_key() with different
- *  encodings, with is a C++ ODR violation.  With current GCC it seems we
- *  luck out, but with clang and MSVC the wrong method gets called for one of
- *  the backends and a corrupt database may result.
- */
-DEFINE_TESTCASE(odrviolation1, glass || chert) {
-    const char* dbname = "odrviolation1";
-    {
-	Xapian::WritableDatabase db = get_named_writable_database(dbname);
-	Xapian::Document document;
-	document.add_value(0, "broken");
-	db.replace_document(16384, document);
-	db.commit();
+/// Regression test for split position handling - broken in 1.4.8.
+DEFINE_TESTCASE(splitpostings1, writable) {
+    Xapian::WritableDatabase db = get_writable_database();
+    Xapian::Document doc;
+    // Add postings to create a split internally.
+    for (Xapian::termpos pos = 0; pos <= 100; pos += 10) {
+	doc.add_posting("foo", pos);
+    }
+    for (Xapian::termpos pos = 5; pos <= 100; pos += 20) {
+	doc.add_posting("foo", pos);
     }
+    db.add_document(doc);
+    db.commit();
 
-    size_t check_errors =
-	Xapian::Database::check(get_named_writable_database_path(dbname),
-				Xapian::DBCHECK_SHOW_STATS, &tout);
-    TEST_EQUAL(check_errors, 0);
+    Xapian::termpos expect = 0;
+    Xapian::termpos pos = 0;
+    for (auto p = db.positionlist_begin(1, "foo");
+	 p != db.positionlist_end(1, "foo"); ++p) {
+	TEST_REL(expect, <=, 100);
+	pos = *p;
+	TEST_EQUAL(pos, expect);
+	expect += 5;
+	if (expect % 20 == 15) expect += 5;
+    }
+    TEST_EQUAL(pos, 100);
 
     return true;
 }
-- 
2.11.4.GIT