Opened 15 years ago
Closed 15 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)
Change History (7)
by , 15 years ago
Attachment: | lazymodproblem.patch added |
---|
comment:1 by , 15 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 , 15 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 , 15 years ago
Component: | Backend-Flint → Library 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 , 15 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I've just committed (in r13914) a documentation comment to describe this situation. Marking as fixed.
comment:5 by , 15 years ago
Milestone: | 1.1.4 → 1.0.18 |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
Reopening - the documentation improvement needs backporting.
comment:6 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Patch to add testcase to the python testsuite which demonstrates this.