Adding support for update_existing_documents to sql and faiss document stores

[lalitpagaria]

Resolve #562

[lalitpagaria]

@tholor and @tanaysoni please review

[tholor]

Thanks for working on this @lalitpagaria !

It seems to me that we are missing one case: When update_existing_documents = True and we call write_documents() with an existing document_id we will update the data in SQL. What about the associated embedding in FAISS? From what I can tell, we will add the new vector in FAISS, add the new vector_id to SQL and leave the "old vector" in FAISS untouched.

I believe deleting single vectors from FAISS is not possible for all index types (https://github.com/facebookresearch/faiss/wiki/Special-operations-on-indexes#removing-elements-from-an-index), but in that case, we should at least log a warning when settingupdate_existing_documents = True and include it in the docstrings.

My second concern would be around speed if update_existing_documents = True as SQL was often the bottleneck before, but I haven't measured it.

What do you think?

[lalitpagaria]

@tholor Yes you are correct, I have added following warning -

if self.update_existing_documents and add_vectors:
            logger.warning("You have enabled `update_existing_documents` feature and "
                           "`FAISSDocumentStore` does not support update in existing `faiss_index`.\n"
                           "Please call `update_embeddings` method to repopulate `faiss_index`")

My second concern would be around speed if update_existing_documents = True as SQL was often the bottleneck before, but I haven't measured it.

We can add this info in doc_string of update_existing_documents parameter, but it is up to user to decide based on their use case.

[lalitpagaria]

Can you please update the doc_string as I am away from my system.

[tanaysoni]

Thank you for working on this, @lalitpagaria. It looks good-to-go!