Opened 14 years ago

Closed 14 years ago

#421 closed defect (fixed)

Document modification can pick up old version

Reported by: Richard Boulton Owned by: Olly Betts
Priority: normal Milestone: 1.0.18
Component: Library API Version: SVN trunk
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Operating System: All

Description

If two copies of a document are read from a database, then one is modified and used to replace the original document, then the other is used to replace the document again (which should put it back to the original value), the document is not reverted as expected, and remains in the state after the modification.

This applies to all backends, but I'm marking it as applying to flint for now, since I have to pick one...

I'm not sure if this _can_ be fixed - I think we'd have to force copies of documents to load their data fully when the original is modified, which in turn would require keeping links to all open document objects in the database. Alternatively, perhaps we could raise an execption (InvalidOperation?) when the document tries to load data from a database which has been modified.

Attachments (1)

lazymodproblem.patch (940 bytes ) - added by Richard Boulton 14 years ago.
Patch to add testcase to the python testsuite which demonstrates this.

Download all attachments as: .zip

Change History (7)

by Richard Boulton, 14 years ago

Attachment: lazymodproblem.patch added

Patch to add testcase to the python testsuite which demonstrates this.

comment:1 by Olly Betts, 14 years ago

I think perhaps we should just document reality (that a Document object is a lazy-fetching facade, so you'll either get the old or new result the corresponding document in the database has been modify) and then the current behaviour is as expected.

It doesn't seem very useful to try to force other user semantics - anything other than your suggestion of detecting this situation and throwing an exception is going to involve disproportionate complexity, and the obvious way to detect modification is just to track and compare the database revision, which is pessimistic as it would prevent reading from documents which aren't modified as well as those that are, so this wouldn't work though is clearly safe (not that it is a particularly important case, though the commit() could be implicit if there have been 9999 batched changes, so this change would add unpredictable failures for this code without the commit()):

Xapian::Document doc1 = db.get_document(1);
Xapian::Document doc2 = db.get_document(2);
doc1.add_term("XTAGfoo");
db.replace_document(1, doc1);
db.commit();
doc2.add_term("XTAGfoo");
db.replace_document(2, doc2);
db.commit();

comment:2 by Richard Boulton, 14 years ago

I think the sensible solution is just to document this. I've just been trying to find a sensible place to document this: it could go in the API comments for Xapian::WritableDatabase::add_document(), but it seems too rare a case to put there really - it's just a distraction from the main point of the API. I've never actually come across this problem in reality - it was just while writing tests for the lazy replace code that I found it.

As far as I know, this problem has been around for a while, so punting it to 1.2.x shouldn't be a problem, in any case. I've also made a note to document this in the manual I'm writing...

comment:3 by Olly Betts, 14 years ago

Component: Backend-FlintLibrary API

Documenting it in the Document class makes most sense to me - this is visible to API manipulation of a Document object too.

Which means that this is really an API issue, not backend so changing component.

We could punt to 1.2.x, but it's just a matter of writing adding a paragraph to a documentation comment, and there's no risk of code regressions, so I'll just do it before 1.1.4.

comment:4 by Richard Boulton, 14 years ago

Resolution: fixed
Status: newclosed

I've just committed (in r13914) a documentation comment to describe this situation. Marking as fixed.

comment:5 by Olly Betts, 14 years ago

Milestone: 1.1.41.0.18
Resolution: fixed
Status: closedreopened

Reopening - the documentation improvement needs backporting.

comment:6 by Olly Betts, 14 years ago

Resolution: fixed
Status: reopenedclosed

Revised documentation comment in trunk r13978 and backported the pair of commits to 1.0 as r13979.

Note: See TracTickets for help on using tickets.