Index: tests/api_wrdb.cc =================================================================== --- tests/api_wrdb.cc (revision 7471) +++ tests/api_wrdb.cc (working copy) @@ -854,6 +854,35 @@ return true; } +// Test replacing a document with itself without modifying postings. +// Regression test for bug in 0.9.9 and earlier - there flint and quartz +// lose all positional information for the document when you do this. +static bool test_replacedoc5() +{ + Xapian::WritableDatabase db = get_writable_database(""); + + Xapian::Document doc1; + doc1.add_posting("hello", 1); + doc1.add_posting("world", 2); + + Xapian::docid did = db.add_document(doc1); + TEST_EQUAL(did, 1); + db.flush(); + + Xapian::Document doc2 = db.get_document(1); + TEST(db.has_positions()); + TEST(db.positionlist_begin(1, "hello") != db.positionlist_end(1, "hello")); + TEST(db.positionlist_begin(1, "world") != db.positionlist_end(1, "world")); + db.replace_document(1, doc2); + db.flush(); + + TEST(db.has_positions()); + TEST(db.positionlist_begin(1, "hello") != db.positionlist_end(1, "hello")); + TEST(db.positionlist_begin(1, "world") != db.positionlist_end(1, "world")); + + return true; +} + // Test of new feature: WritableDatabase::replace_document and delete_document // can take a unique termname instead of a document id as of Xapian 0.8.2. static bool test_uniqueterm1() @@ -1034,6 +1063,7 @@ {"replacedoc2", test_replacedoc2}, {"replacedoc3", test_replacedoc3}, {"replacedoc4", test_replacedoc4}, + {"replacedoc5", test_replacedoc5}, {"uniqueterm1", test_uniqueterm1}, {"phraseorneartoand1", test_phraseorneartoand1}, {"longpositionlist1", test_longpositionlist1}, Index: backends/inmemory/inmemory_database.cc =================================================================== --- backends/inmemory/inmemory_database.cc (revision 7471) +++ backends/inmemory/inmemory_database.cc (working copy) @@ -480,7 +480,6 @@ ++posting; } } - termlists[did - 1] = InMemoryDoc(); doclengths[did - 1] = 0; doclists[did - 1] = document.get_data(); @@ -515,6 +514,7 @@ add_values(did, values); } + InMemoryDoc doc; Xapian::TermIterator i = document.termlist_begin(); Xapian::TermIterator i_end = document.termlist_end(); for ( ; i != i_end; ++i) { @@ -527,11 +527,11 @@ if (j == j_end) { /* Make sure the posting exists, even without a position. */ - make_posting(*i, did, 0, i.get_wdf(), false); + make_posting(&doc, *i, did, 0, i.get_wdf(), false); } else { positions_present = true; for ( ; j != j_end; ++j) { - make_posting(*i, did, *j, i.get_wdf()); + make_posting(&doc, *i, did, *j, i.get_wdf()); } } @@ -541,6 +541,7 @@ postlists[*i].collection_freq += i.get_wdf(); ++postlists[*i].term_freq; } + swap(termlists[did - 1], doc); totdocs++; } @@ -563,12 +564,14 @@ return termlists.size(); } -void InMemoryDatabase::make_posting(const string & tname, +void InMemoryDatabase::make_posting(InMemoryDoc * doc, + const string & tname, Xapian::docid did, Xapian::termpos position, Xapian::termcount wdf, bool use_position) { + Assert(doc); Assert(postlists.find(tname) != postlists.end()); Assert(did > 0 && did <= termlists.size()); Assert(did > 0 && did <= doclengths.size()); @@ -595,7 +598,7 @@ termentry.wdf = wdf; // Now record the termentry - termlists[did - 1].add_posting(termentry); + doc->add_posting(termentry); } bool Index: backends/inmemory/inmemory_database.h =================================================================== --- backends/inmemory/inmemory_database.h (revision 7471) +++ backends/inmemory/inmemory_database.h (working copy) @@ -220,7 +220,8 @@ void finish_add_doc(Xapian::docid did, const Xapian::Document &document); void add_values(Xapian::docid did, const map &values_); - void make_posting(const string & tname, + void make_posting(InMemoryDoc * doc, + const string & tname, Xapian::docid did, Xapian::termpos position, Xapian::termcount wdf, Index: backends/quartz/quartz_database.cc =================================================================== --- backends/quartz/quartz_database.cc (revision 7471) +++ backends/quartz/quartz_database.cc (working copy) @@ -1000,14 +1000,13 @@ j->second.insert(make_pair(did, make_pair('A', wdf))); } - // FIXME : this might not work if we replace a positionlist - // with itself (e.g. if a document is replaced with itself - // with just the values changed) - database_ro.positionlist_table.delete_positionlist(did, tname); - if (term.positionlist_begin() != term.positionlist_end()) { + PositionIterator it = term.positionlist_begin(); + PositionIterator it_end = term.positionlist_end(); + if (it != it_end) { database_ro.positionlist_table.set_positionlist( - did, tname, - term.positionlist_begin(), term.positionlist_end()); + did, tname, it, it_end); + } else { + database_ro.positionlist_table.delete_positionlist(did, tname); } } } Index: backends/remote/remote-database.cc =================================================================== --- backends/remote/remote-database.cc (revision 7471) +++ backends/remote/remote-database.cc (working copy) @@ -199,6 +199,7 @@ bool RemoteDatabase::has_positions() const { + if (!cached_stats_valid) update_stats(); return has_positional_info; } Index: backends/flint/flint_database.cc =================================================================== --- backends/flint/flint_database.cc (revision 7471) +++ backends/flint/flint_database.cc (working copy) @@ -856,14 +856,13 @@ j->second.insert(make_pair(did, make_pair('A', wdf))); } - // FIXME : this might not work if we replace a positionlist - // with itself (e.g. if a document is replaced with itself - // with just the values changed) - database_ro.positionlist_table.delete_positionlist(did, tname); - if (term.positionlist_begin() != term.positionlist_end()) { + PositionIterator it = term.positionlist_begin(); + PositionIterator it_end = term.positionlist_end(); + if (it != it_end) { database_ro.positionlist_table.set_positionlist( - did, tname, - term.positionlist_begin(), term.positionlist_end()); + did, tname, it, it_end); + } else { + database_ro.positionlist_table.delete_positionlist(did, tname); } } }